On Thu, Aug 25, 2005 at 05:24:05PM -0400, Alan DeKok wrote:
bugzilla-daemon@dump.segv.org wrote:
------- Additional Comments From nbk@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@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/ -----------------------------------------------------------