Paul TBBle Hampson <Paul.Hampson@Pobox.com> wrote:
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.
Sure.
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 >=3D0 which is called against a uint8_t* here.
Yeah, it's annoying. I'm looking at that code now, and will see if I can fix it. I don't think it has any bad side-effects, but it should be fixed.
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.
Yes.
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)
Yes.
And I then suggest that the above is more like HEAD work than RELEASE_1_0 work.
Yes. Alan DeKok.