The usage of strtol in this case always wanted a base-10 number, so to "fix"
this particular case we want:
while (isspace(*str & 255))
str ++;
if (*str == '+' || *str == '-')
str ++;
while (isdigit(*str & 255))
str ++;
However, this has always been documented as "Cnnn" - decimal number, no sign,
whitespace, etc. allowed - and the original "fix" should be sufficient...
(The inline function solution, while effective, is a bit too "hacky" for my
taste...)
On Oct 18, 2011, at 3:53 PM, Albrecht Schlosser wrote:
> On 18.10.2011 20:43, Michael Sweet wrote:
>> In this case I would not use strtol to skip digits, but a simple while loop
>> instead:
>>
>> while (isdigit(*str& 255))
>> str ++;
>>
>> (the "& 255" part is necessary to avoid portability issues with UTF-8
>> strings)
>
> Nice idea, but strtol() would also skip white space (at the beginning of
> the string), an optional '+' or '-' sign, a "0x" prefix, and also
> use hex digits, if base is 16...
>
> Honestly said, I don't know which number formats FLTK supports when
> parsing Fl_Browser items - just to leave the code as-is I added an
> inline function to replace strtol() for this special case. This
> function fools the compiler in that it returns the return value of
> strtol(), but is not "declared with attribute warn_unused_result".
>
> Maybe strtol() was (and is) the wrong function in the first place,
> but I'm not going to check the docs now (is it documented at all?).
>
> This is the patch:
>
> --- snip ---
> Index: Fl_Browser.cxx
> ===================================================================
> --- Fl_Browser.cxx (revision 9137)
> +++ Fl_Browser.cxx (working copy)
> @@ -56,6 +56,18 @@
> char txt[1]; // start of allocated array
> };
>
> +/*
> + static inline helper function to skip a (color) number in a string.
> + This is to avoid a (gcc) compiler warning "ignoring return value of
> + long int strtol(const char*, char**, int), declared with attribute
> + warn_unused_result". This could also be achieved by a while loop
> + skipping digits, but strtol() would also skip leading white space,
> + a sign, and maybe a "0x" prefix.
> +*/
> +inline static long int skip_num(const char *nptr, char **endptr, int
> base) {
> + return strtol(nptr,endptr,base);
> +}
> +
> /**
> Returns the very first item in the list.
> Example of use:
> @@ -372,7 +384,6 @@
> if (hh > hmax) hmax = hh;
> } else {
> const int* i = column_widths();
> - long int dummy;
> // do each column separately as they may all set different fonts:
> for (char* str = l->txt; str && *str; str++) {
> Fl_Font font = textfont(); // default font
> @@ -387,7 +398,7 @@
> case 'i': font = (Fl_Font)(font|FL_ITALIC); break;
> case 'f': case 't': font = FL_COURIER; break;
> case 'B':
> - case 'C': dummy = strtol(str, &str, 10); break;// skip a color
> number
> + case 'C': skip_num(str, &str, 10); break;// skip a color number
> case 'F': font = (Fl_Font)strtol(str,&str,10); break;
> case 'S': tsize = strtol(str,&str,10); break;
> case 0: case '@': str--;
> @@ -440,7 +451,6 @@
> int done = 0;
>
> while (*str == format_char_ && str[1] && str[1] != format_char_) {
> - long int dummy;
> str ++;
> switch (*str++) {
> case 'l': case 'L': tsize = 24; break;
> @@ -450,7 +460,7 @@
> case 'i': font = (Fl_Font)(font|FL_ITALIC); break;
> case 'f': case 't': font = FL_COURIER; break;
> case 'B':
> - case 'C': dummy = strtol(str, &str, 10); break;// skip a color number
> + case 'C': skip_num(str, &str, 10); break;// skip a color number
> case 'F': font = (Fl_Font)strtol(str, &str, 10); break;
> case 'S': tsize = strtol(str, &str, 10); break;
> case '.':
> @@ -535,7 +545,6 @@
> //#warning FIXME This maybe needs to be more UTF8 aware now...?
> //#endif /*__GNUC__*/
> while (*str == format_char() && *++str && *str != format_char()) {
> - long int dummy;
> switch (*str++) {
> case 'l': case 'L': tsize = 24; break;
> case 'm': case 'M': tsize = 18; break;
> @@ -549,7 +558,7 @@
> if (!(l->flags & SELECTED)) {
> fl_color((Fl_Color)strtol(str, &str, 10));
> fl_rectf(X, Y, w1, H);
> - } else dummy = strtol(str, &str, 10);
> + } else skip_num(str, &str, 10);
> break;
> case 'C':
> lcol = (Fl_Color)strtol(str, &str, 10);
> --- snip ---
>
> Advantages:
> (1) No dummy variables.
> (2) No runtime overhead, since static and inline.
>
> Any comments?
>
> Albrecht
> _______________________________________________
> fltk-dev mailing list
> fltk-dev@easysw.com
> http://lists.easysw.com/mailman/listinfo/fltk-dev
_____________
Michael Sweet
_______________________________________________
fltk-dev mailing list
fltk-dev@easysw.com
http://lists.easysw.com/mailman/listinfo/fltk-dev
