Hi,

I did a static analysis of ed 1.12 with cppcheck and found 3 issues:

    [buffer.c:447]: (error) Memory leak: lp
    [io.c:264]: (error) Resource leak: fp
    [io.c:322]: (error) Resource leak: fp

The two ressource leaks were a file not closed on error.
The memory leak is a line node allocated but not freed on error in
put_sbuf_line. I'll admit that as global variables are involved, I'm
not completely sure that it is a memory leak, but it sure looks like
so.

You will find with this mail two patches that seem to correct the
problems. Once applied, cppcheck doesn't complain anymore and all
tests pass.

Honestly, I can't see how those bugs could become problematic in any
realistic way but bugs are meant to be fixed, aren't they?

Regards,

Cédric Picard

PS: as this is my first contribution to a GNU project, feel free to
tell me if I can improve anything.
--- ../ed-1.12.reference/buffer.c       2015-06-07 19:10:25.000000000 +0200
+++ buffer.c    2015-07-17 22:28:38.280753869 +0200
@@ -444,7 +444,12 @@
   int len;
 
   if( !lp ) return 0;
-  if( !p ) { set_error_msg( "Line too long" ); return 0; }
+  if( !p )
+    {
+        set_error_msg( "Line too long" );
+        free( lp );
+        return 0;
+    }
   len = p - buf;
   /* out of position */
   if( seek_write )
@@ -453,6 +458,7 @@
       {
       show_strerror( 0, errno );
       set_error_msg( "Cannot seek temp file" );
+      free( lp );
       return 0;
       }
     sfpos = ftell( sfp );
@@ -463,6 +469,7 @@
     sfpos = -1;
     show_strerror( 0, errno );
     set_error_msg( "Cannot write temp file" );
+    free( lp );
     return 0;
     }
   lp->pos = sfpos; lp->len = len;
--- ../ed-1.12.reference/io.c   2015-01-11 19:43:16.000000000 +0100
+++ io.c        2015-07-17 22:29:04.640930193 +0200
@@ -261,8 +261,8 @@
     return -1;
     }
   size = read_stream( fp, addr );
-  if( size < 0 ) return -1;
   if( *filename == '!' ) ret = pclose( fp ); else ret = fclose( fp );
+  if( size < 0 ) return -1;
   if( ret != 0 )
     {
     show_strerror( filename, errno );
@@ -319,8 +319,8 @@
     return -1;
     }
   size = write_stream( fp, from, to );
-  if( size < 0 ) return -1;
   if( *filename == '!' ) ret = pclose( fp ); else ret = fclose( fp );
+  if( size < 0 ) return -1;
   if( ret != 0 )
     {
     show_strerror( filename, errno );
_______________________________________________
bug-ed mailing list
[email protected]
https://lists.gnu.org/mailman/listinfo/bug-ed

Reply via email to