Type of yy_n_chars changed to yy_size_t causes warnings with comparison to zero

[srivasta]

Hi,

This was reported by a Debian user in Bug 770161

In version 2.5.35 the type of yy_n_chars is int, while in 2.5.39 the type is yy_size_t. The yy_size_t is internal type mapped to size_t which is unsigned. The YY_INPUT macro contains following code:

#define YY_INPUT(buf,result,max_size) \
    errno=0; \
    while ( (result = read( fileno(yyin), (char *) buf, max_size )) < 0 ) \
    { \
            if( errno != EINTR) \

then the macro is then invoked:

YY_INPUT( (&YY_CURRENT_BUFFER_LVALUE->yy_ch_buf[number_to_move]),
                    yyg->yy_n_chars, num_to_read );

So the code casts signed result of read() to unsigned and then compare if it's lower than 0.

Manoj

Here is how to reproduce:

credit.l.txt

flex -Cr credit.l
_> gcclint lex.yy.c
<>

lex.yy.c:1203:3: note: in expansion of macro ‘YY_INPUT’
   YY_INPUT( (&YY_CURRENT_BUFFER_LVALUE->yy_ch_buf[number_to_move]),
   ^
lex.yy.c:781:20: warning: conversion to ‘yy_size_t {aka long unsigned int}’ from ‘ssize_t {aka long int}’ may change the sign of the result [-Wsign-conversion]
  while ( (result = read( fileno(yyin), (char *) buf, max_size )) < 0 ) \
                    ^
lex.yy.c:1203:3: note: in expansion of macro ‘YY_INPUT’
   YY_INPUT( (&YY_CURRENT_BUFFER_LVALUE->yy_ch_buf[number_to_move]),
   ^
lex.yy.c:781:66: warning: comparison of unsigned expression < 0 is always false [-Wtype-limits]
  while ( (result = read( fileno(yyin), (char *) buf, max_size )) < 0 ) \
                                                                  ^
lex.yy.c:1203:3: note: in expansion of macro ‘YY_INPUT’
   YY_INPUT( (&YY_CURRENT_BUFFER_LVALUE->yy_ch_buf[number_to_move]),
   ^
lex.yy.c: In function ‘input’:
lex.yy.c:1363:23: warning: conversion to ‘yy_size_t {aka long unsigned int}’ from ‘long int’ may change the sign of the result [-Wsign-conversion]
    yy_size_t offset = (yy_c_buf_p) - (yytext_ptr);
                       ^

[westes]

I see that on line 178 of src/flexdef.h, yy_n_chars is declared with typ int.

Any objections if I close this issue given that?

[srivasta]

Hi,

Hmm. I still see yy_n_chars only defined in src/FlexLexer.h and src/flex.skl; and the latter still defines it as yy_size_t. I just pulled froim master. Am I missing something?

Manoj

_> rgrep -i yy_n_chars src
src/FlexLexer.h:  int yy_n_chars;
src/flex.skl:   yy_size_t yy_n_chars;
src/flex.skl:static yy_size_t yy_n_chars;               /* number of characters read into yy_ch_buf */
src/flex.skl:    yy_size_t yy_n_chars;
src/flex.skl:                   YY_G(yy_n_chars) = YY_CURRENT_BUFFER_LVALUE->yy_n_chars;
src/flex.skl:           if ( YY_G(yy_c_buf_p) <= &YY_CURRENT_BUFFER_LVALUE->yy_ch_buf[YY_G(yy_n_chars)] )
src/flex.skl:                           &YY_CURRENT_BUFFER_LVALUE->yy_ch_buf[YY_G(yy_n_chars)];
src/flex.skl:   if ( YY_G(yy_c_buf_p) > &YY_CURRENT_BUFFER_LVALUE->yy_ch_buf[YY_G(yy_n_chars) + 1] )
src/flex.skl:           YY_CURRENT_BUFFER_LVALUE->yy_n_chars = YY_G(yy_n_chars) = 0;
src/flex.skl:                   YY_G(yy_n_chars), num_to_read );
src/flex.skl:           YY_CURRENT_BUFFER_LVALUE->yy_n_chars = YY_G(yy_n_chars);
src/flex.skl:   if ( YY_G(yy_n_chars) == 0 )
src/flex.skl:   if ((yy_size_t) (YY_G(yy_n_chars) + number_to_move) > YY_CURRENT_BUFFER_LVALUE->yy_buf_size) {
src/flex.skl:           yy_size_t new_size = YY_G(yy_n_chars) + number_to_move + (YY_G(yy_n_chars) >> 1);
src/flex.skl:   YY_G(yy_n_chars) += number_to_move;
src/flex.skl:   YY_CURRENT_BUFFER_LVALUE->yy_ch_buf[YY_G(yy_n_chars)] = YY_END_OF_BUFFER_CHAR;
src/flex.skl:   YY_CURRENT_BUFFER_LVALUE->yy_ch_buf[YY_G(yy_n_chars) + 1] = YY_END_OF_BUFFER_CHAR;
src/flex.skl:           yy_size_t number_to_move = YY_G(yy_n_chars) + 2;
src/flex.skl:           YY_CURRENT_BUFFER_LVALUE->yy_n_chars =
src/flex.skl:                   YY_G(yy_n_chars) = YY_CURRENT_BUFFER_LVALUE->yy_buf_size;
src/flex.skl:           if ( YY_G(yy_c_buf_p) < &YY_CURRENT_BUFFER_LVALUE->yy_ch_buf[YY_G(yy_n_chars)] )
src/flex.skl:           YY_CURRENT_BUFFER_LVALUE->yy_n_chars = YY_G(yy_n_chars);
src/flex.skl:   YY_G(yy_n_chars) = YY_CURRENT_BUFFER_LVALUE->yy_n_chars;
src/flex.skl:   b->yy_n_chars = 0;
src/flex.skl:           YY_CURRENT_BUFFER_LVALUE->yy_n_chars = YY_G(yy_n_chars);
src/flex.skl:   b->yy_n_chars = b->yy_buf_size;

[srivasta]

The example file (credit.l) still creates a .c file that runs into the gcc diagnostics. Perhaps I built from the wrong branch, I'll check when I get back home

Manoj

[westes]

Ah, I only noticed the flexlexer.h declaration. The flex.skl stuff needs to change.

[westes]

Also, what's the gcclint command you're using? I'm not finding that as a standard command.

[westes]

As per sf #160, I reproduced the error with "gcc -Wtype-limits". The commit 4a8eccf removes the warning sited in this report. The change is in master and will be included in the next release.

[srivasta]

Sorry. I should have mentioned what I use for linting:

#!/bin/bash 
if [ -x /bin/tempfile ]; then
    gfile=`tempfile -p tcap -m 0600 `.o;
else
    set -e
    mkdir /tmp/glint$$
    gfile=/tmp/glint$$/gfile.o
fi
# -std=gnu99  
exec gcc -std=c99 -pedantic -Wextra  -Wall -Wformat=2 -Wno-format-extra-args \
         -Winit-self  -Wmissing-include-dirs -Wswitch-default -Wswitch-enum  \
         -Wunused -Wstrict-overflow=5 -Wfloat-equal -Wundef -Wno-endif-labels\
         -Wshadow -Wunsafe-loop-optimizations  -Wpointer-arith               \
         -Wbad-function-cast -Wc++-compat -Wcast-qual -Wcast-align           \
         -Wwrite-strings -Wconversion -Wlogical-op -Waggregate-return        \
         -Wpacked -Wpadded -Wunreachable-code -Winline -Wvariadic-macros     \
         -Wvla -Wvolatile-register-var  -Wstack-protector                    \
         -Woverlength-strings -Wmissing-declarations -Wmissing-parameter-type\
         -Wmissing-prototypes  -Wnested-externs -Wold-style-declaration      \
         -Wold-style-definition -Wstrict-prototypes -Wpointer-sign           \
         -fshort-enums -fno-common -fstack-protector                         \
         -D_FORTIFY_SOURCE=2 -Dgets=DONT_USE_GETS -Dlint -O2 -g              \
          -o $gfile -c $*


if test -f $gfile; then
   rm -f $gfile
fi
if [ -d /tmp/glint$$ ]; then
    rmdir /tmp/glint$$
fi