Date: Tue, 22 Jun 2010 03:22:40 +1000 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Marcel Moolenaar <marcel@freebsd.org> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r209358 - head/cddl/contrib/opensolaris/lib/libdtrace/common Message-ID: <20100622024652.C43995@delplex.bde.org> In-Reply-To: <201006200034.o5K0Y6xl041024@svn.freebsd.org> References: <201006200034.o5K0Y6xl041024@svn.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sun, 20 Jun 2010, Marcel Moolenaar wrote: > Log: > Unbreak platforms with char unsigned by default. Oddly enough, GCC isn't > satisfied with a simple cast to int in the check against EOF, so the fix > is a bit involved by actually having to go through a temporary variable. Perhaps that is because gcc can see that the cast has no effect, so the comparison can never be true if `c' is an unsigned char (unless unsigned char has the same number of bits as signed int), but it cannot see that the conversion to the temporary variable has the same null effect. > Modified: head/cddl/contrib/opensolaris/lib/libdtrace/common/dt_lex.l > ============================================================================== > --- head/cddl/contrib/opensolaris/lib/libdtrace/common/dt_lex.l Sat Jun 19 22:13:40 2010 (r209357) > +++ head/cddl/contrib/opensolaris/lib/libdtrace/common/dt_lex.l Sun Jun 20 00:34:06 2010 (r209358) > @@ -67,8 +67,12 @@ > * for all subsequent invocations, which is the effect desired. > */ > #undef unput > -#define unput(c) \ > - if (c != EOF) yyunput( c, yytext_ptr ) > +#define unput(c) \ > + do { \ > + int _c = c; \ > + if (_c != EOF) \ > + yyunput(_c, yytext_ptr); \ > + } while(0) > #endif > > static int id_or_type(const char *); > This remains broken, especially on platforms with chars unsigned. No one should try to unput EOF, so c should never equal EOF, but if c is a negative character it may equal EOF and thus On platforms with chars unsigned (except exotic ones where chars have the same size as ints), if c is a char then it is >= 0 and thus cannot equal EOF (which is < 0). Since the platform is non-exotic, (int)c and "int _c = c;" equal c and are this still >= 0 and thus cannot equal EOF. Thus the comparison with EOF has no effect, and c is always unput. On platforms with chars signed, some chars may equal EOF. It is an error to unput almost any value held in a variable of type char, since its value might equal EOF and thus be rejected by unput(), but unput() should be able to handle any character in the character set. This problem is handled by ungetc() by always converting the value to unsigned char. Thus the value can never equal EOF, and the character set is effectively represented by unsigned char's, not the plain chars that stdio returns in some other interfaces (but not getc()). There seems to be no reason to break the warning about this instead of using the same approach as stdio. This depends on yyunput() not having similar bugs (it must take an arg of type int and convert to an unsigned cgar like ungetc()): #define unput(c) yyunput((unsigned char)(c), yytext_ptr) This also fixes the missing parantheses for 'c' and some style bugs. Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20100622024652.C43995>