Skip site navigation (1)Skip section navigation (2)
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>