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

Re: libdvdread: Patch OK?



On Mon, 2011-09-05 at 00:23 +0200, Pino Toscano wrote:
> Alle domenica 4 settembre 2011, Svante Signell ha scritto:
> > Does this patch look OK, or is it totally wrong?

> Some time ago I made the attached one, but never got around polishing an 
> sending it. Should do a bit more allocations than the code using 
> PATH_MAX-sized arrays, but also quite less OS-specific code paths to 
> maintain (the __GLIBC__ one is valid for basically almost all the Linux 
> developers, so won't rot easily).

Attached is a modified version of your patch also taking Samuels
comments into account. Probably still not completely OK?

--- ./libdvdread-4.1.4-1219/src/dvd_reader.c	2010-07-31 02:10:28.000000000 +0200
+++ ./libdvdread-4.1.4-1219.modified/src/dvd_reader.c	2011-09-05 15:20:53.000000000 +0200
@@ -424,6 +424,12 @@
         if( chdir( path_copy ) == -1 ) {
           goto DVDOpen_error;
         }
+#ifdef __GLIBC__
+        new_path = get_current_dir_name();
+        if(new_path == NULL) {
+          goto DVDOpen_error;
+        }
+#else
         new_path = malloc(PATH_MAX+1);
         if(!new_path) {
           goto DVDOpen_error;
@@ -431,6 +437,7 @@
         if( getcwd( new_path, PATH_MAX ) == NULL ) {
           goto DVDOpen_error;
         }
+#endif
         retval = fchdir( cdir );
         close( cdir );
         cdir = -1;
@@ -625,17 +632,19 @@
  *     or -1 on file not found.
  *     or -2 on path not found.
  */
-static int findDirFile( const char *path, const char *file, char *filename )
+static int findDirFile( const char *path, const char *file, char **filename )
 {
   DIR *dir;
   struct dirent *ent;
+  *filename = NULL;
 
   dir = opendir( path );
   if( !dir ) return -2;
 
   while( ( ent = readdir( dir ) ) != NULL ) {
     if( !strcasecmp( ent->d_name, file ) ) {
-      sprintf( filename, "%s%s%s", path,
+      *filename = malloc( strlen( path ) + 1 + strlen( ent->d_name ) + 1 );
+      sprintf( *filename, "%s%s%s", path,
                ( ( path[ strlen( path ) - 1 ] == '/' ) ? "" : "/" ),
                ent->d_name );
       closedir(dir);
@@ -646,9 +655,9 @@
   return -1;
 }
 
-static int findDVDFile( dvd_reader_t *dvd, const char *file, char *filename )
+static int findDVDFile( dvd_reader_t *dvd, const char *file, char **filename )
 {
-  char video_path[ PATH_MAX + 1 ];
+  char *video_path = NULL;
   const char *nodirfile;
   int ret;
 
@@ -659,19 +668,22 @@
     nodirfile = file;
   }
 
-  ret = findDirFile( dvd->path_root, nodirfile, filename );
+  ret = findDirFile( dvd->path_root, nodirfile, &filename );
   if( ret < 0 ) {
     /* Try also with adding the path, just in case. */
+    video_path = malloc( strlen( dvd->path_root ) + 10 + 1 );
     sprintf( video_path, "%s/VIDEO_TS/", dvd->path_root );
-    ret = findDirFile( video_path, nodirfile, filename );
+    ret = findDirFile( video_path, nodirfile, &filename );
     if( ret < 0 ) {
       /* Try with the path, but in lower case. */
       sprintf( video_path, "%s/video_ts/", dvd->path_root );
-      ret = findDirFile( video_path, nodirfile, filename );
+      ret = findDirFile( video_path, nodirfile, &filename );
       if( ret < 0 ) {
+        free( video_path );
         return 0;
       }
     }
+    free( video_path );
   }
 
   return 1;
@@ -682,20 +694,22 @@
  */
 static dvd_file_t *DVDOpenFilePath( dvd_reader_t *dvd, char *filename )
 {
-  char full_path[ PATH_MAX + 1 ];
+  char *full_path = NULL;
   dvd_file_t *dvd_file;
   struct stat fileinfo;
   dvd_input_t dev;
 
   /* Get the full path of the file. */
-  if( !findDVDFile( dvd, filename, full_path ) ) {
+  if( !findDVDFile( dvd, filename, &full_path ) ) {
     fprintf( stderr, "libdvdnav:DVDOpenFilePath:findDVDFile %s failed\n", filename );
+    free( full_path );
     return NULL;
   }
 
   dev = dvdinput_open( full_path );
   if( !dev ) {
     fprintf( stderr, "libdvdnav:DVDOpenFilePath:dvdinput_open %s failed\n", full_path );
+    free( full_path );
     return NULL;
   }
 
@@ -703,6 +717,7 @@
   if( !dvd_file ) {
     fprintf( stderr, "libdvdnav:DVDOpenFilePath:dvd_file malloc failed\n" );
     dvdinput_close(dev);
+    free( full_path );
     return NULL;
   }
   dvd_file->dvd = dvd;
@@ -714,6 +729,7 @@
 
   if( stat( full_path, &fileinfo ) < 0 ) {
     fprintf( stderr, "libdvdread: Can't stat() %s.\n", filename );
+    free( full_path );
     free( dvd_file );
     return NULL;
   }
@@ -721,6 +737,7 @@
   dvd_file->title_devs[ 0 ] = dev;
   dvd_file->filesize = dvd_file->title_sizes[ 0 ];
 
+  free( full_path );
   return dvd_file;
 }
 
@@ -776,7 +793,7 @@
 static dvd_file_t *DVDOpenVOBPath( dvd_reader_t *dvd, int title, int menu )
 {
   char filename[ MAX_UDF_FILE_NAME_LEN ];
-  char full_path[ PATH_MAX + 1 ];
+  char *full_path = NULL;
   struct stat fileinfo;
   dvd_file_t *dvd_file;
   int i;
@@ -799,13 +816,15 @@
     } else {
       sprintf( filename, "VTS_%02i_0.VOB", title );
     }
-    if( !findDVDFile( dvd, filename, full_path ) ) {
+    if( !findDVDFile( dvd, filename, &full_path ) ) {
+      free( full_path );
       free( dvd_file );
       return NULL;
     }
 
     dev = dvdinput_open( full_path );
     if( dev == NULL ) {
+      free( full_path );
       free( dvd_file );
       return NULL;
     }
@@ -813,6 +832,7 @@
     if( stat( full_path, &fileinfo ) < 0 ) {
       fprintf( stderr, "libdvdread: Can't stat() %s.\n", filename );
       dvdinput_close(dev);
+      free( full_path );
       free( dvd_file );
       return NULL;
     }
@@ -825,7 +845,7 @@
     for( i = 0; i < TITLES_MAX; ++i ) {
 
       sprintf( filename, "VTS_%02i_%i.VOB", title, i + 1 );
-      if( !findDVDFile( dvd, filename, full_path ) ) {
+      if( !findDVDFile( dvd, filename, &full_path ) ) {
         break;
       }
 
@@ -840,11 +860,12 @@
       dvd_file->filesize += dvd_file->title_sizes[ i ];
     }
     if( !dvd_file->title_devs[ 0 ] ) {
+      free( full_path );
       free( dvd_file );
       return NULL;
     }
   }
-
+  free( full_path );
   return dvd_file;
 }
 
@@ -966,7 +987,7 @@
                                int menu, dvd_stat_t *statbuf )
 {
   char filename[ MAX_UDF_FILE_NAME_LEN ];
-  char full_path[ PATH_MAX + 1 ];
+  char *full_path = NULL;
   struct stat fileinfo;
   off_t tot_size;
   off_t parts_size[ 9 ];
@@ -978,11 +999,13 @@
   else
     sprintf( filename, "VTS_%02d_%d.VOB", title, menu ? 0 : 1 );
 
-  if( !findDVDFile( dvd, filename, full_path ) )
+  if( !findDVDFile( dvd, filename, &full_path ) )
+    free( full_path );
     return -1;
 
   if( stat( full_path, &fileinfo ) < 0 ) {
     fprintf( stderr, "libdvdread: Can't stat() %s.\n", filename );
+    free( full_path );
     return -1;
   }
 
@@ -994,7 +1017,7 @@
     int cur;
     for( cur = 2; cur < 10; cur++ ) {
       sprintf( filename, "VTS_%02d_%d.VOB", title, cur );
-      if( !findDVDFile( dvd, filename, full_path ) )
+      if( !findDVDFile( dvd, filename, &full_path ) )
         break;
 
       if( stat( full_path, &fileinfo ) < 0 ) {
@@ -1013,6 +1036,7 @@
   for( n = 0; n < nr_parts; n++ )
     statbuf->parts_size[ n ] = parts_size[ n ];
 
+  free( full_path );
   return 0;
 }
 
@@ -1021,7 +1045,7 @@
                  dvd_read_domain_t domain, dvd_stat_t *statbuf )
 {
   char filename[ MAX_UDF_FILE_NAME_LEN ];
-  char full_path[ PATH_MAX + 1 ];
+  char *full_path = NULL;
   struct stat fileinfo;
   uint32_t size;
 
@@ -1077,17 +1101,19 @@
       return 0;
     }
   } else {
-    if( findDVDFile( dvd, filename, full_path ) ) {
+    if( findDVDFile( dvd, filename, &full_path ) ) {
       if( stat( full_path, &fileinfo ) < 0 )
         fprintf( stderr, "libdvdread: Can't stat() %s.\n", filename );
       else {
         statbuf->size = fileinfo.st_size;
         statbuf->nr_parts = 1;
         statbuf->parts_size[ 0 ] = statbuf->size;
+        free( full_path );
         return 0;
       }
     }
   }
+  free( full_path );
   return -1;
 }
 

Reply to: