[libdvdnav-devel] [RFC] Set free'd variables to NULL or not?

[Andrew Clayton]

I notice it is common practice in libdvdread to set free'd variables
to NULL. There are of course arguments for and against.

However there are currently situations where a variable is being set to
NULL after being free'd that was passed in by value to the function and
thus setting it to NULL has no effect outside the function.

E.g in src/dvd_read.h in DVDCloseFile(), dvd_file is passed in by
value, free'd and then set to NULL, however that has no effect outside
this function.

Another example is in src/ifo_read.c in  ifoClose() which has basically
the same thing.

The simple solution is to just remove these NULL settings. If however
the NULL setting is wanted to be propagated then for the dvd_read.c
case the patch looks something like.

 src/dvd_reader.c         | 20 ++++++++++----------
 src/dvdread/dvd_reader.h |  2 +-
 src/ifo_read.c           |  2 +-
 3 files changed, 12 insertions(+), 12 deletions(-)

So the question is what do we want to do here?

Cheers,
Andrew

[Patch below just for show]

diff --git a/src/dvd_reader.c b/src/dvd_reader.c
index fb045b3..8da58d9 100644
--- a/src/dvd_reader.c
+++ b/src/dvd_reader.c
@@ -932,21 +932,21 @@ dvd_file_t *DVDOpenFile( dvd_reader_t *dvd, int titlenum,
   }
 }
 
-void DVDCloseFile( dvd_file_t *dvd_file )
+void DVDCloseFile( dvd_file_t **dvd_file )
 {
   int i;
 
-  if( dvd_file && dvd_file->dvd ) {
diff --git a/src/dvd_reader.c b/src/dvd_reader.c
index fb045b3..8da58d9 100644
--- a/src/dvd_reader.c
+++ b/src/dvd_reader.c
@@ -932,21 +932,21 @@ dvd_file_t *DVDOpenFile( dvd_reader_t *dvd, int titlenum,
   }
 }
 
-void DVDCloseFile( dvd_file_t *dvd_file )
+void DVDCloseFile( dvd_file_t **dvd_file )
 {
   int i;
 
-  if( dvd_file && dvd_file->dvd ) {
-    if( !dvd_file->dvd->isImageFile ) {
+  if( *dvd_file && (*dvd_file)->dvd ) {
+    if( !(*dvd_file)->dvd->isImageFile ) {
       for( i = 0; i < TITLES_MAX; ++i ) {
-        if( dvd_file->title_devs[ i ] ) {
-          dvdinput_close( dvd_file->title_devs[i] );
+        if( (*dvd_file)->title_devs[ i ] ) {
+          dvdinput_close( (*dvd_file)->title_devs[i] );
         }
       }
     }
 
-    free( dvd_file );
-    dvd_file = NULL;
+    free( *dvd_file );
+    *dvd_file = NULL;
   }
 }
 
@@ -1393,7 +1393,7 @@ int DVDDiscID( dvd_reader_t *dvd, unsigned char *discid )
       char *buffer = (char *)(((uintptr_t)buffer_base & ~((uintptr_t)2047)) + 2
 
       if( buffer_base == NULL ) {
-          DVDCloseFile( dvd_file );
+          DVDCloseFile( &dvd_file );
           fprintf( stderr, "libdvdread: DVDDiscId, failed to "
                    "allocate memory for file read!\n" );
           return -1;
@@ -1403,14 +1403,14 @@ int DVDDiscID( dvd_reader_t *dvd, unsigned char *discid 
       if( bytes_read != file_size ) {
           fprintf( stderr, "libdvdread: DVDDiscId read returned %zd bytes"
                    ", wanted %zd\n", bytes_read, file_size );
-          DVDCloseFile( dvd_file );
+          DVDCloseFile( &dvd_file );
           free( buffer_base );
           return -1;
       }
 
       md5_process_bytes( buffer, file_size,  &ctx );
 
-      DVDCloseFile( dvd_file );
+      DVDCloseFile( &dvd_file );
       free( buffer_base );
       nr_of_files++;
     }
diff --git a/src/dvdread/dvd_reader.h b/src/dvdread/dvd_reader.h
index 0e6d395..673456e 100644
--- a/src/dvdread/dvd_reader.h
+++ b/src/dvdread/dvd_reader.h
@@ -174,7 +174,7 @@ dvd_file_t *DVDOpenFile( dvd_reader_t *, int, dvd_read_domai
  *
  * DVDCloseFile(dvd_file);
  */
-void DVDCloseFile( dvd_file_t * );
+void DVDCloseFile( dvd_file_t ** );
 
 /**