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;
}
