[Dnsmasq-discuss] pending works before 2.57

[Simon Kelley]

Matthias Andree wrote:
> Am 16.02.2011 10:51, schrieb Simon Kelley:
>> Ben wrote:
>>> So after upgrading from 2.55 to 2.56 I am getting the following error at
>>> startup:
>>>
>>>       dnsmasq: error at line 3 of /etc/dnsmasq.d/options.conf
>>>
>>> Line 3 in that file is:
>>>
>>>       dhcp-mac=set:special,00:e0:91:*:*:*
>>>
>>> So, this has worked happily for a while, but certainly w/ 2.55.  Has
>>> there been a change in that command or is this a bug in 2.56?
>>>
>> Bug. Sorry. Extra sanity checking that's a bit too zealous. The attached
>> patch fixes it. I'll give it a few more days for any more nasties in
>> 2.56 to hatch and then re-release. Must find time to write a regression
>> test suite.
> 
> Simon,
> 
> I have some issues:
> 
> #1 ...and before 2.57, please audit all tolower/toupper() and is*() 
> ctype.h functions for proper casts.
> 
> The code currently reads:
> 
>    char *r;
>    ...
>    if (*r != '*' && !isxdigit((int)*r))
> 
> But should be ...!isxdigit((unsigned char)*r)) - mind the cast. Using 
> (int) is extraneous and doesn't have any effect, but instead, it gives 
> you the wrong sign extension and undefined behaviour on systems where 
> the char type is not unsigned for *r between -128 and -2 inclusively.
> 
> Quoting SUSv4: "The c argument is an int, the value of which the 
> application shall ensure is a character representable as an unsigned 
> char or equal to the value of the macro EOF. If the argument has any 
> other value, the behavior is undefined." -- and that's what you get by 
> casting to unsigned char (you can't get EOF here because it's not 
> representable in a char without aliasing 0xFF -- that's useful however 
> to apply the is*() functions to fgetc() results and thereabouts.)
> 
> 

A good point. I've fixed these now.


> 
> #2 I reported that I18N and IDN support are inseparable build options 
> against 2.51 already.
> 
> Do you have plans to allow for individual switches?  I can fancy people 
> desiring IDN but not I18N. Count me in :)

I'm wary of starting to omit IDN unless a new preprocessor is defined.
That will bite existing users. It would be OK to make IDN conditional on
I18N OR -DHAVE_IDN, so that people who want only IDN can get that, but
existing builds that do I18N still get IDN without making changes.

> 
> 
> #3 I'm also wondering if the stuff above shouldn't rather be 
> !ishexnumber() for locale compatibility.
> 

Is ishexnumber portable? Anyone who uses locale-specific characters to
specify MAC addresses is asking for a whole heap of trouble, IMHO.

Cheers,

Simon.

> Best regards
>