bug-ed
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Antonio Diaz Diaz
Subject: Re: [bug-ed] BUGs: static code analysis (gcc, clang, cppcheck)
Date: Wed, 10 Mar 2021 12:28:00 +0100
User-agent: Mozilla/5.0 (X11; U; Linux i586; en-US; rv:1.9.1.19) Gecko/20110420 SeaMonkey/2.0.14

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.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]