Re: [bug-ed] BUGs: static code analysis (gcc, clang, cppcheck)

[Antonio Diaz Diaz]

Hola Xose,

Thanks for your report.

Xose Vazquez Perez wrote:
> signal.c: In function 'sighup_handler.part.0':
> signal.c:57:9: warning: leak of '<unknown>' [CWE-401] [-Wanalyzer-malloc-leak]
> 57 | if( len && hup ) /* hup filename */
>    |   ^

The code in question is this:

      if( len && hup )                  /* hup filename */
        {
        memcpy( hup, s, len );
        if( need_slash ) hup[len] = '/';
        memcpy( hup + len + need_slash, hb, sizeof hb );
        if( write_file( hup, "w", 1, last_addr() ) >= 0 ) exit( 0 );
        }
      exit( 1 );                        /* hup file write failed */


Not freeing the hup pointer does not cause any leak because ed is going to 
exit in any case. But I have reorganized the code with the hope to make it 
more obvious to static analyzers.


> carg_parser.c:239:27: warning: Potential leak of memory pointed to by
> 'non_options' [unix.Malloc]
> if( !tmp ) return 0;
>                   ^

Same as above. The command line argument parser exhausted memory and the 
program can't continue. But again I have "fixed" it.


> global.c:94:10: warning: Access to field 'q_forw' results in a
> dereference of a null pointer (loaded from variable 'bp') [core.NullDereference]
> bp = bp->q_forw;
>      ^~~~~~~~~~

This one (and the rest) seem to be false positives. 'bp' is always a valid 
line node. I don't see where is the null pointer.


> signal.c:183:21: warning: Array access (from variable 'buf') results in
> a null pointer dereference [core.NullDereference]
> while( ( buf[i++] = ( (*p == '\\' ) ? *++p : *p ) ) )
>          ~~~      ^

This is the code:

  static char * buf = 0;
  static int bufsz = 0;
  const int len = strlen( p );
  int i = 0;

  if( !resize_buffer( &buf, &bufsz, len + 1 ) ) return 0;
  /* assert: no trailing escape */
  while( ( buf[i++] = ( (*p == '\\' ) ? *++p : *p ) ) )
    ++p;
  return buf;


Maybe the analyzer is not clever enough to note that 'resize_buffer' 
initializes 'buf'?


> buffer.c:577:9: error: Memory pointed to by 'ustack' is freed twice. 
> [doubleFree]
> free( ustack );
> ^
> buffer.c:568:28: note: Memory pointed to by 'ustack' is freed twice.
> if( ustack ) new_buf = realloc( ustack, new_size );
>                        ^

Once again the analyzer does not seem clever enough to note that 'realloc' 
does not free 'ustack' if the reallocation fails.


Best regards,
Antonio.