#20 Some fixes to skel.c

closed-rejected
Will Estes
5
2014-07-15
2006-05-28
Anonymous
No

Hi,

Thanks for maintaining the flex port. I used it with
Thomas Niemann's calc interpreter to make a simple
interpreter to do interactive tests of some library
functions. I modified flex-2.5.33/skel.c to avoid some
compiler and valgrind warnings. I think some of these
are duplicated in existing bug reports. I'm sorry but I
don't know enough about automake/autoconf/m4 etc to fix
the macros directly.

Cheers,

Rob Jenssen

1. Compiler warnings. The following is the compiler
output for calc3a from Thomas Niemann's "A Compact
Guide to Lex and Yacc";

$ gcc -v
Using built-in specs.
Configured with: FreeBSD/i386 system compiler
Thread model: posix
gcc version 3.4.2 [FreeBSD] 20040728

$ bison -V
bison (GNU Bison) 2.1
Written by Robert Corbett and Richard Stallman.

Copyright (C) 2005 Free Software Foundation, Inc.
This is free software; see the source for copying
conditions. There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A
PARTICULAR PURPOSE.

$ /usr/local/bin/flex -V
flex 2.5.33

$/usr/local/bin/bison -y -d calc3.y

$/usr/local/bin/flex calc3.l

$gcc -Wall -Wshadow -Wpointer-arith -Wsign-compare
-Wstrict-prototypes -g -O0 -c y.tab.c lex.yy.c
lex.yy.c: In function `yy_get_next_buffer':
lex.yy.c:1029: warning: comparison between signed and
unsigned
calc3.l: At top level:
lex.yy.c:1121: warning: 'yyunput' defined but not used

$gcc -Wall -Wshadow -Wpointer-arith -Wsign-compare
-Wstrict-prototypes -O0 -g y.tab.o lex.yy.o calc3a.c -o
calc3a

2. Memory leaks because yyin isn't closed. Running
valgrind:

$ echo "x=2;printx;"| valgrind --leak-check=yes
--show-reachable=yes --num-callers=16 calc3a
==46816== Memcheck, a memory error detector for x86-linux.
==46816== Copyright (C) 2002-2004, and GNU GPL'd, by
Julian Seward.
==46816== Using valgrind-2.1.0, a program supervision
framework for x86-linux.
==46816== Copyright (C) 2000-2004, and GNU GPL'd, by
Julian Seward.
==46816== Estimated CPU clock rate is 2921 MHz
==46816== For more details, rerun with: -v
==46816==
2

==46816==
==46816== ERROR SUMMARY: 0 errors from 0 contexts
(suppressed: 0 from 0)
==46816== malloc/free: in use at exit: 4096 bytes in 1
blocks.
==46816== malloc/free: 9 allocs, 8 frees, 20594 bytes
allocated.
==46816== For counts of detected errors, rerun with: -v
==46816== searching for pointers to 1 not-freed blocks.
==46816== checked 1294188 bytes.
==46816==
==46816== 4096 bytes in 1 blocks are still reachable in
loss record 1 of 1
==46816== at 0x3C03E183: malloc (in
/usr/local/lib/valgrind/vgpreload_memcheck.so)
==46816== by 0x3C0F7341: __smakebuf (in /lib/libc.so.5)
==46816== by 0x3C0E4F92: __srefill (in /lib/libc.so.5)
==46816== by 0x3C0E005E: fread (in /lib/libc.so.5)
==46816== by 0x8049E2A: yy_get_next_buffer
(lex.yy.c:1029)
==46816== by 0x8049942: yylex (lex.yy.c:871)
==46816== by 0x8048BB8: yyparse (y.tab.c:1099)
==46816== by 0x804953C: main (calc3.y:157)
==46816==
==46816== LEAK SUMMARY:
==46816== definitely lost: 0 bytes in 0 blocks.
==46816== possibly lost: 0 bytes in 0 blocks.
==46816== still reachable: 4096 bytes in 1 blocks.
==46816== suppressed: 0 bytes in 0 blocks.

3. I didn't attempt to debug the m4 scripts that
generate skel.c. Here is a patch file:

--- flex-2.5.33/skel.c Tue Feb 21 13:45:41 2006
+++ skel.c Sun May 28 15:30:42 2006
@@ -556,8 +556,7 @@
"]])",
"",
- "m4_ifdef( [[M4_YY_NOT_IN_HEADER]],",
- "[[",
+ "#ifndef YY_NO_UNPUT",
"#define unput(c) yyunput( c, YY_G(yytext_ptr)
M4_YY_CALL_LAST_ARG )",
- "]])",
+ "#endif",
"",
"/* The following is because we cannot portably get
our hands on size_t",
@@ -1020,8 +1019,7 @@
"",
"%not-for-header",
- " m4_ifdef( [[M4_YY_NO_UNPUT]],,",
- " [[",
+ "#ifndef YY_NO_UNPUT",
" static void yyunput M4_YY_PARAMS( int c, char
*buf_ptr M4_YY_PROTO_LAST_ARG);",
- " ]])",
+ "#endif",
"%ok-for-header",
"%endif",
@@ -1790,5 +1788,5 @@
" else",
" {",
- " int num_to_read =",
+ " size_t num_to_read =",
"
YY_CURRENT_BUFFER_LVALUE->yy_buf_size - number_to_move
- 1;",
"",
@@ -1920,7 +1918,6 @@
"",
"",
+ "#ifndef YY_NO_UNPUT",
"%if-c-only",
- "m4_ifdef( [[M4_YY_NO_UNPUT]],,",
- "[[",
" static void yyunput YYFARGS2( int,c, register
char *,yy_bp)",
"%endif",
@@ -1972,7 +1969,5 @@
" YY_G(yy_c_buf_p) = yy_cp;",
"}",
- "%if-c-only",
- "]])",
- "%endif",
+ "#endif",
"",
"%if-c-only",
@@ -2980,4 +2975,5 @@
" yyout = stdout;",
"#else",
+ " fclose(yyin);",
" yyin = (FILE *) 0;",
" yyout = (FILE *) 0;",

The patch was created with:

diff -U 2 flex-2.5.33/skel.c skel.c > skel.c.patch

and can be applied with:

cd flex-2.5.33
patch<../skel.c.patch

Discussion

  • Will Estes
    Will Estes
    2006-10-19

    • assigned_to: nobody --> wlestes
     
  • Will Estes
    Will Estes
    2006-10-20

    Logged In: YES
    user_id=595627

    We are reviewing this patch.

     
  • Will Estes
    Will Estes
    2006-10-27

    • status: open --> closed-rejected
     
  • Will Estes
    Will Estes
    2006-10-27

    Logged In: YES
    user_id=595627

    The submitted patch made two sorts of changes:

    * changing the internal markup in the scanner skeleton. That's very bad since our
    build/generation process depends on having conditional statements read in a particular way.

    * changed the type of a variable from int to size_t. That in and of itself is fine, but
    I'm not convinced at this point that the variable in question is always non-negative.
    (It's pretty clear that this ought to be the case, but we'll be better off doing a
    wholesale audit of such concerns rather than cherry-picking at random.)