On Tue, Oct 04, 2005 at 10:04:14PM +0200, Rdiger Plm wrote:
> >> P.S. The reverse case is sillier, given that the value is moving to a
> >> *larger* type and therefore no data loss can occur:
> >>
> >> short a;
> >> long b = a; // (7) no warning
> >>
> >> short a;
> >> void *b = a; // (8) warns "pointer from integer without a
> >> cast" (ok)
> >> void *c = (void *)a; // (9) warns "cast to pointer from integer of
> >> different size"
> >> void *d = (void *)(long)a; // (10) even more stupid!!!
> >>
> >> Note that having a 'large enough' integral type in case (10) is not good
> >> enough. We need to cast 'a' to an integral type which is *exactly* the
> >> same
> >> size as a void *, to silence this particular warning.
> >
> >
> > I agree with your assertion that the code example above IS a gcc bug.
> > This is expected in C++, but the waning (9) above is completely bogus,
>
> I do not think so. While "a" does fit in "c" from the storage point of view
> converting "c" to a different pointer type e.g. (char *) and dereferencing it
> afterwards most likely leads to SIGBUS or SIGSEGV. So I think a warning is
> justified here.
I disagree. For example, if you take a (char *) and cast it to a (void *),
and later to an (int *), you will very likely get your SIGBUS error, but the
compiler won't warn you about it. That's because (void *) is explicitly
meant to be "a pointer to anything you choose". Once you've cast anything
through a void *, you lose any type checking of the thing pointed to.
If you're casting an integer to a pointer, as Apache does: then almost
certainly the pointer isn't going to be usable for deferencing (unless the
value you're casting from happened to hold an integer representation of
another pointer). Furthermore, we're casting to a void *, and you can't
dereference through a void * anyway.
Now, Apache is arguably abusing void * by using it as a data type for
holding arbitrary integers. One assumption is that those integers (like
pid_t) are all no larger than a void *. That's a reasonable assumption on
most platforms (OK, TurboC excepted :-)
Once we agree on that assumption, then there is no need to see a warning
when casting a small integer type into a larger void *, or a void * back
into a smaller integer type. So I say it's the warning that's the problem,
not the code.
> What's your though on AP_PTRINT(n) / AP_INTPTR(p)?
Well, I suppose we're always going to have to have a cast if moving an int
into a void * or vice versa. So:
pid = AP_PTRINT(ap_hash_get(....))
works as well as anything. I would vote having the second level of cast
though: that is, not
pid = (pid_t)AP_INTPTR(ap_hash_get(....))
If we're using AP_PTRINT then I think we have to assume the compiler won't
give you a warning if assigning a 64-bit integral value into a 32-bit
variable. That seems to work for gcc, right now. But if it doesn't in
future, we would then need macros like
AP_PTR_PID_T(n)
AP_PID_T_PTR(p)
Ergh.
IMO this still boils down to: gcc is giving a code warning which is
inappropriate/irrelevant for the coding style of the Apache project. Hell,
just *document* that this warning is generated all over the place, and that
anyone building the code should ignore it. Then the code can say
pid = (pid_t)ap_hash_get(...)
which is what was meant all along.
Cheers,
Brian.
