[DVDnav-discuss] [PATCH 1/6] Some system calls that report errors are not being checked.

Reimar Döffinger Reimar.Doeffinger at stud.uni-karlsruhe.de
Fri Feb 13 20:32:42 CET 2009


On Fri, Feb 13, 2009 at 11:20:49AM -0800, Erik Hovland wrote:
> diff --git a/src/dvd_reader.c b/src/dvd_reader.c
> index 65900f7..66289e7 100644
> --- a/src/dvd_reader.c
> +++ b/src/dvd_reader.c
> @@ -421,14 +421,29 @@ dvd_reader_t *DVDOpen( const char *ppath )
>        int cdir = open( ".", O_RDONLY );
>  
>        if( cdir >= 0 ) {
> -        chdir( path_copy );
> +        if ( chdir( path_copy ) == -1 ) {
> +          close( cdir );
> +          return NULL;
> +        }
>          new_path = malloc(PATH_MAX+1);
>          if(!new_path) {
>            free(path);
>            return NULL;
>          }
> -        getcwd(new_path, PATH_MAX );
> -        fchdir( cdir );
> +        if ( getcwd( new_path, PATH_MAX ) == NULL ) {
> +          free( path );
> +          close( cdir );
> +          free( path_copy );
> +          free( new_path );
> +          return NULL;
> +        }
> +        if ( fchdir( cdir ) == -1 ) {
> +          free( path );
> +          close( cdir );
> +          free( path_copy );
> +          free( new_path );
> +          return NULL;
> +        }
>          close( cdir );
>            free( path_copy );
>            path_copy = new_path;

Obviously there are a whole lot of free/close etc. missing, and with
more returns that IMO becomes completely unmaintainable.
I think this is a perfect place for something like "goto error" which
does all the necessary cleanup.
Afterwards the error checks can be added.



More information about the DVDnav-discuss mailing list