linux-sparse.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Josh Triplett <josh@freedesktop.org>
To: Al Viro <viro@ftp.linux.org.uk>
Cc: linux-sparse@vger.kernel.org,
	Linus Torvalds <torvalds@linux-foundation.org>
Subject: Re: fun with ?:
Date: Tue, 22 May 2007 14:40:11 -0700	[thread overview]
Message-ID: <4653633B.3000000@freedesktop.org> (raw)
In-Reply-To: <20070519025249.GZ4095@ftp.linux.org.uk>

[-- Attachment #1: Type: text/plain, Size: 3462 bytes --]

Al Viro wrote:
> 	There's an unpleasant case in conditional operator we are getting
> wrong.
> 	int *p;
> 	const void *v;
> 	int n;
> 
> 	n ? p : (const void *)0
> 
> According to C standard, the type of that expression is const void *.  Note
> that
> 	n ? p : (void *)0
> is an entirely different story - it's int *.

That much actually makes sense to me.  You can convert from (int *) to (const
void *), but not from (const void *) to (int *), so I'd expect the behavior of
the first case.  You *can* convert bidirectionally between (void *) and (int
*), so I expect the behavior of the second case as well.

> What's going on here is pretty simple: there are two degenerate cases of
> conditional operator: pointer vs. null pointer constant and pointer vs.
> possibly qualified pointer to void.  Look at these cases:
> 	n ? p : NULL => should be the same type as p
> 	n ? p : v => clearly const void * - pointer to void with union of
> qualifiers; in this case we obviously lose any information about the type
> of object being pointed to.

I didn't actually know about the special case for a null pointer constant.

> The tricky part comes from definition of what null pointer constant _is_.
> C allows two variants - integer constant expression with value 0 (we accept
> it, but warn about bad taste) and the same cast to void * (we also accept
> that, of course).

Right.

> Note that this is specific type - pointer to void.  Without any qualifiers.
> We are guaranteed that we can convert it to any pointer type and get
> a pointer distinct from address of any object.  So (const void *)0 is the same
> thing as (const void *)(void *)0 and it is the null pointer to const void.
> *HOWEVER*, it is not a null pointer constant.  The standard is clear here and
> frankly, it's reasonable.  If you cast to anything other than void *, then
> you presumably mean it and want the conversion rules as for any pointer
> of that type.  Think of something like
> #ifdef FOO
> const void *f(int n);
> #else
> #define f(n) ((const void *)NULL)
> #endif
> You don't want to have types suddenly change under you depending on FOO.

Definitely not.  I don't want qualifiers to disappear just because I applied
them to NULL.

> sparse is more liberal than standard C in what it accepts as null pointer
> constant.  It almost never matters; however, in case of conditional operator
> we end up with a different type for an expression both sparse and any
> C compiler will accept as valid.
>
> I'm fixing other fun stuff in that area (e.g. we ought to take a union of
> qualifiers, ought _not_ to mix different structs or unions, etc.), so
> unless there are serious objections I'd rather go with standard behaviour
> in that case.  What will change:
> 
> int n;
> int *p;
> 
> n ? p : (const void *)NULL	int *	=>	const void *
> n ? p : (const void *)0		ditto
> n ? p : (char *)0		int *	=>	a warning on mixing int * with							char *
> n ? p : (char *)NULL		ditto
> n ? p : (void *)NULL		int *	=>	void *
> n ? p : (void *)0		unchanged
> n ? p : NULL			unchanged
> 
> Objections?

Thanks for the clear explanation.  I think following the standard seems like a
good idea here, though I find some of the cases somewhat unintuitive and
potentially error-prone.

In particular:
> n ? p : (void *)NULL		int *	=>	void *
Shouldn't this have type int * just like n ? p : NULL ?

- Josh Triplett


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 252 bytes --]

  reply	other threads:[~2007-05-22 21:40 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-05-19  2:52 fun with ?: Al Viro
2007-05-22 21:40 ` Josh Triplett [this message]
2007-05-22 22:46   ` Al Viro
2007-05-22 23:24     ` Josh Triplett
2007-05-23  0:02       ` Al Viro
2007-05-23  0:25         ` Al Viro
2007-05-23  1:05           ` Josh Triplett
2007-05-23  4:53           ` Al Viro
2007-05-23 12:26             ` Morten Welinder
2007-05-23  1:03         ` Josh Triplett
2007-06-03  1:05           ` Al Viro
2007-05-23 14:25         ` Neil Booth
2007-05-23 14:32           ` Al Viro
2007-05-23 14:47             ` Neil Booth
2007-05-23 15:32               ` Al Viro
2007-05-23 23:01                 ` Neil Booth
2007-05-24  0:10                   ` Derek M Jones
2007-05-24  0:14                   ` Al Viro
2007-05-23 21:16             ` Derek M Jones
2007-05-23 21:59               ` Linus Torvalds
2007-05-23 23:29                 ` Derek M Jones
2007-05-24  0:02                   ` Al Viro
2007-05-24  0:29                   ` Linus Torvalds
2007-05-24  1:36               ` Brett Nash

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4653633B.3000000@freedesktop.org \
    --to=josh@freedesktop.org \
    --cc=linux-sparse@vger.kernel.org \
    --cc=torvalds@linux-foundation.org \
    --cc=viro@ftp.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).