git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Linus Torvalds <torvalds@osdl.org>
To: Horst von Brand <vonbrand@inf.utfsm.cl>
Cc: git@vger.kernel.org
Subject: Re: [PATCH] Trivial warning fix for imap-send.c
Date: Mon, 13 Mar 2006 08:26:56 -0800 (PST)	[thread overview]
Message-ID: <Pine.LNX.4.64.0603130759320.3618@g5.osdl.org> (raw)
In-Reply-To: <200603130414.k2D4EXcX011651@laptop11.inf.utfsm.cl>



On Mon, 13 Mar 2006, Horst von Brand wrote:
>> 
> > This breaks down with variadic functions, which have no typing
> > information. So doing this:
> >   execl("foo", "bar", my_struct_foo);
> > doesn't give the compiler a chance to do the implicit cast and you get
> > subtle breakage (in the same way that you would if you passed a long to
> > a variadic function expecting a short).
> 
> It just passes 3 "void *"'s, and casts back. What is so strange?

It doesn't actually pass 3 "void *".

Variadic functions pass their arguments as-is (apart from the normal 
integer promotion for small integer types: chars are passed as integers if 
"char" is smaller than "int").

So no actual casting takes place for the arguments, and

	execl("foo", "bar", my_struct_foo, NULL);

passes in four pointers: two of type "char *", one of whatever 
my_struct_foo is ("struct foo *") and finally hopefully one of "void *" 
(modulo broken compilers). They might - in theory - have different sizes 
and representations.

Now, the interesting effect of this is that simple things like

	printf("Pointer value %p\n", myfunction);

is actually NOT STRICTLY PORTABLE CODE! Why? Because "%p" wants a void 
pointer, but "myfunction" is of a different pointer type, which may 
actually have a different size and a different representation entirely, so 
you can get total garbage printed out.

So you have two choices:

 - be sane, and ignore insane architectures. In this case, the above is 
   perfectly fine C code.

 - be insane, and care about it. In this case, you really do have to add 
   the casts to be safe in theory.

The "%p" for printf() is actually a wonderful example of why you really 
really really should ignore language lawyers. According to language 
lawyers, you should add that "(void *)" cast. But look around for how many 
such casts you can find in real code, and realize that the language 
lawyers just don't matter. A C compiler environment that requires it is 
simply broken, and sane people will refuse to use it for anything than 
small embedded work, because it's simply not usable.

So while it's not true in theory, in _practice_ you should expect all 
pointers to be of the same size and use the same representation. Anything 
else is just too painful to be ever worth bothering with.

(Remember near and far pointers from 16-bit DOS/Windows? Those 
environments at least had _explicit_ pointer sizes, making the problem 
less horrible, but that was clearly a disaster nonetheless. Pointers that 
implicitly have different sizes because they point to different types are 
even _worse_).

			Linus

PS. Final words: while knowing that all pointer representations must be 
the same, and that NULL actually must be the bitwise binary "all zero" 
value (otherwise "memset(structptr, 0, structsize)" would break), it's 
equally important to know that the standard doesn't _guarantee_ it. Why? 

Because that way, when somebody tries to tell you that your code isn't 
standards conformant and might be unportable, you can say "yeah, I know, 
but only a total loser would ever care". Instead of incorrectly trying to 
argue that your code is "correct".

  reply	other threads:[~2006-03-13 16:27 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-03-11 19:29 [PATCH] Trivial warning fix for imap-send.c Art Haas
2006-03-12 10:44 ` Mark Wooding
2006-03-12 11:27   ` Junio C Hamano
2006-03-12 13:59     ` [PATCH] Use explicit pointers for execl...() sentinels Mark Wooding
2006-03-12 15:13       ` Timo Hirvonen
2006-03-12 17:32         ` Mark Wooding
2006-03-12 18:08           ` Timo Hirvonen
2006-03-13  3:31             ` Jeff King
2006-03-13  4:12               ` Horst von Brand
2006-03-14  0:42                 ` [OT] " Jeff King
2006-03-12 16:57   ` [PATCH] Trivial warning fix for imap-send.c Linus Torvalds
2006-03-12 18:01     ` Mark Wooding
2006-03-12 19:20       ` A Large Angry SCM
2006-03-13  2:59         ` H. Peter Anvin
2006-03-13  4:36           ` A Large Angry SCM
2006-03-13  5:22             ` Linus Torvalds
2006-03-13  6:37               ` H. Peter Anvin
2006-03-13  6:46                 ` Linus Torvalds
2006-03-13 16:37                 ` Olivier Galibert
2006-03-13  3:38         ` Jeff King
2006-03-13  4:14           ` Horst von Brand
2006-03-13 16:26             ` Linus Torvalds [this message]
2006-03-13  6:41           ` H. Peter Anvin
2006-03-12 21:51       ` Horst von Brand
2006-03-12 23:02       ` Linus Torvalds

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=Pine.LNX.4.64.0603130759320.3618@g5.osdl.org \
    --to=torvalds@osdl.org \
    --cc=git@vger.kernel.org \
    --cc=vonbrand@inf.utfsm.cl \
    /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).