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
[email protected]
http://lists.easysw.com/mailman/listinfo/fltk-dev

Reply via email to