[Bug 269] Many compiler warnings with gcc 4.0

Paul TBBle Hampson Paul.Hampson at Pobox.com
Tue Aug 30 18:51:07 CEST 2005


On Thu, Aug 25, 2005 at 05:24:05PM -0400, Alan DeKok wrote:
> bugzilla-daemon at dump.segv.org wrote:
>> ------- Additional Comments From nbk at sitadelle.com  2005-08-24 09:58 -------
>> Many thanks for second patch! I've added it to the CVS branch 1.0,
>> so your fixes will be in release 1.0.5.
>> 
>> There're still problems of incompatible pointer types, but I need
>> Alan's opinion about bringing so many casts in the
>> source. (personally I don't like the idea)

>   Neither do I.  They make the code harder to read & maintain.

>   Alan DeKok.

A brief glance through the patch in the bugreport indicates a lot of the casts
were for value_pair's strvalue member from uint8_t* to char*...

What might solve this better is if we can somehow get char to default to
unsigned when building, and all these will go away.

Also, hidden amongst the signedness errors are occasional "comparison always
<blah> due to limited range". One such (that I've looked at before and given up
on) is src/lib/radius.c:1439. This comes about because the definition of
TAG_VALID_ZERO contains two tests, one of which is >=0 which is called against
a uint8_t* here.

The other two uses of TAG_VALID_ZERO are in valuepair.c, and are made with an
explicitly signed char coming out of strtol (which is defined here as returning
a long int...! Is this a big nasty bug????) and which could maybe be replaced
with strtoul, since a return <0 is discarded. However, strtoul appears to
discard any negatiion, rather than truncating at 0, so this last may not be
much use.

Possibly large swathes of the server could be cleaned up to use uint8_t instead
of char, and such warnings could be eliminated, but we will still get
signedness problems from the libc calls which are defined with char parameters,
and char is considered signed.

I agree that the cast is ugly, and in fact a few prototypes (such as
rad_pwdecode) could be changed to take a uint8_t instead of a char to remove
some similar casts.  (Speculation based on the block directly above
src/lib/radius.c:1439)

And I then suggest that the above is more like HEAD work than RELEASE_1_0 work.
^_^

-- 
-----------------------------------------------------------
Paul "TBBle" Hampson, MCSE
8th year CompSci/Asian Studies student, ANU
The Boss, Bubblesworth Pty Ltd (ABN: 51 095 284 361)
Paul.Hampson at Pobox.Com

Of course Pacman didn't influence us as kids. If it did,
we'd be running around in darkened rooms, popping pills and
listening to repetitive music.
 -- Kristian Wilson, Nintendo, Inc, 1989

License: http://creativecommons.org/licenses/by/2.1/au/
-----------------------------------------------------------
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://lists.freeradius.org/pipermail/freeradius-devel/attachments/20050831/a5862850/attachment.pgp>


More information about the Freeradius-Devel mailing list