misc/143350: pointer comparison against '\0' in strfmon.c

[Corinna Vinschen]

>Number:         143350
>Category:       misc
>Synopsis:       pointer comparison against '\0' in strfmon.c
>Confidential:   no
>Severity:       serious
>Priority:       low
>Responsible:    freebsd-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          sw-bug
>Submitter-Id:   current-users
>Arrival-Date:   Fri Jan 29 19:40:09 UTC 2010
>Closed-Date:
>Last-Modified:
>Originator:     Corinna Vinschen
>Release:        CVS HEAD
>Organization:
Cygwin
>Environment:
>Description:
There appears to be a bug in strfom.c, function __setup_vars.

When the value for the negative sign string is fetched, there
are two comparisons of the following style:

  *signstr = (lc->negative_sign == '\0') ? "-"
      : lc->negative_sign;

lc->negative_sign is a string pointer.  The above statement
compares the pointer against '\0'.  This looks like a typo.
Actually the comparison should check if the first character
in lc->negative_sign is \0, or better, if lc->negative_sign
is an empty string.  A check for NULL doesn't make sense
since localeconv never returns NULL pointers as struct lconv
members.

So I think the patch below should be the right way to go.


Corinna
>How-To-Repeat:

>Fix:
Index: strfmon.c
===================================================================
RCS file: /home/ncvs/src/lib/libc/stdlib/strfmon.c,v
retrieving revision 1.19
diff -u -p -r1.19 strfmon.c
--- strfmon.c   24 Apr 2008 07:49:00 -0000      1.19
+++ strfmon.c   29 Jan 2010 19:23:39 -0000
@@ -413,7 +413,7 @@ __setup_vars(int flags, char *cs_precede
                *cs_precedes = lc->int_n_cs_precedes;
                *sep_by_space = lc->int_n_sep_by_space;
                *sign_posn = (flags & PARENTH_POSN) ? 0 : lc->int_n_sign_posn;
-               *signstr = (lc->negative_sign == '\0') ? "-"
+               *signstr = (*lc->negative_sign == '\0') ? "-"
                    : lc->negative_sign;
        } else if (flags & USE_INTL_CURRENCY) {
                *cs_precedes = lc->int_p_cs_precedes;
@@ -424,7 +424,7 @@ __setup_vars(int flags, char *cs_precede
                *cs_precedes = lc->n_cs_precedes;
                *sep_by_space = lc->n_sep_by_space;
                *sign_posn = (flags & PARENTH_POSN) ? 0 : lc->n_sign_posn;
-               *signstr = (lc->negative_sign == '\0') ? "-"
+               *signstr = (*lc->negative_sign == '\0') ? "-"
                    : lc->negative_sign;
        } else {
                *cs_precedes = lc->p_cs_precedes;


>Release-Note:
>Audit-Trail:
>Unformatted: