httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Brian Candler <B.Cand...@pobox.com>
Subject Re: svn commit: r293305 - in /httpd/httpd/branches/2.2.x/modules:
Date Tue, 04 Oct 2005 19:52:03 GMT
On Tue, Oct 04, 2005 at 12:12:14PM -0400, Jim Jagielski wrote:
> Brian Candler wrote:
> > 
> > Hmm. Given the example
> > 
> >     pid = (pid_t)((long)apr_hash_get(script_hash, &cgid_req.conn_id, sizeof(cgid_req.conn_id)));
> > 
> > then it's no good declaring variable 'pid' to be of type apr_intptr_t rather
> > than pid_t, because somewhere down the line it will be passed to a function
> > which expects a pid_t argument, and you'll get a warning there instead. Are
> > you saying it should be written as
> > 
> 
> The above line just confuses me, but I haven't taken the time
> to try to understand the rationale for why it's written the way it is.

That's what I was hoping the little example with shorts, longs and void *'s
would explain :-)

In the above code:

- apr_hash_get returns void *
- if you assign it directly to 'pid', you get a compiler warning saying
  "assigning integer from pointer without a cast". That's fair enough.
- however, if you cast it to pid_t, you get a compiler warning saying
  "cast from pointer to integer of different size", if pid_t is not
  actually the same size as a void *

So stupidly you have to cast it to an integer of the same size as a pointer,
*then* cast it to pid_t or whatever you wanted in the first place, to
silence the warnings.

I think this is the compiler grumbling about something which it shouldn't,
at least for Apache's way of working: the code explicitly casts void * to a
smaller integral type, because it was being used to hold a value of a
smaller integral type in the first place. IMO the cast is the programmer's
way of saying "yes, I know this is a pointer, and yes I want you treat it as
a pid_t (or whatever)". I guess the warning is for the benefit of people who
use an integral type for temporarily holding a pointer, rather than a
pointer type for temporarily holding an integer.

I believe the solution is to silence the compiler warnings, rather than
corrupt the code with superfluous casts. A union might be cleaner, in which
case you'd write something like

    pid = apr_hash_get(...).num;

I would hope that most modern compilers are able to cope with passing and
returning unions by value (especially ones which fit within a machine word),
but maybe that doesn't apply to all the platforms Apache is supposed to
compile on.

Regards,

Brian.

// Proof of concept
#include <stdio.h>

typedef union {
  long long_v;
  void *void_p;
} ap_union;

ap_union foo(ap_union bar)
{
    return bar;
}

int main(void)
{
    ap_union x, y;
    x.void_p = "hello";
    
    y = foo(x);
    printf("%s\n", (char *)y.void_p);
    printf("%08lx\n", y.long_v);
    return 0;
}

Mime
View raw message