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 16:05:10 GMT
On Mon, Oct 03, 2005 at 09:33:55AM -0400, Jim Jagielski wrote:
> > Joe's correct that this code change works on ILP32, ILP64 and LP64, 
> > platforms - but I concur with Jim that for the casual developer, the
> > purpose is hard to glean...
> > 
> > ...perhaps we need an apr_intptr_t type which is a best-fit int for any
> > arbitrary void* storage class?
> > 
> 
> ANSI C specifically allows for pointers to be converted to
> a "large enough" integral type. For safety, I would suggest unsigned
> long, but whenever we have impl dependent code, we should make
> sure it's well documented, abstract it out if possible, or, at
> least, confirm during configuration that our assumptions are
> correct.

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

    pid = (pid_t)(apr_intptr_t)apr_hash_get(...)

instead? Or just as

    pid = (apr_intptr_t)apr_hash_get(...)

and rely on the compiler not moaning if pid_t is smaller than apr_intptr_t?

ISTM the fundamental problem is illustrated here:

    long a;
    short b = a;         // (1) a warning would be useful here
    short c = (short)a;  // (2) no warning

    void *a;
    short b = a;         // (3) warns "integer from pointer without a cast" (ok)
    short c = (short)a;  // (4) warns "cast from pointer to integer of different size"

so you have to write

    short d = (short)(long)a;   // (5) stupid!!!

which is just ridiculous, or possibly

    short e = (long)a;          // (6) ??

which looks like the programmer might have made a mistake. It doesn't seem
to issue a warning at present (as tested with gcc 3.4.2), but I expect it
might in the future, by analogy with case (1).

As I see it, if I explicitly cast something to a (foo) then I'm contracting
with the compiler that I'm happy to have the value truncated to fit, as in
case (2), so I don't see that the compiler should warn in case (4).

Now, Apache has lots of cases where (void *)'s are cast to integral types
(possibly smaller than a void *) and back again. Since this is intentional,
I think the real solution is to turn off the compiler warning for this case,
not to bastardise the code with lots of double-casts. The more unnecessary
casts you put into your code, the more likely you are to make type errors
which the compiler can't check.

IMO, if gcc can't disable this warning, then we need to file a bug report.
Or else just pipe gcc stderr through

    egrep -v "warning: cast (to|from) pointer (to|from) integer of different size"

and forget about it.

Regards,

Brian.

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.

Mime
View raw message