[vlc-devel] [PATCH v2 4/13] esout: refactor GetTmpFile()

[Lyndon Brown]

On Tue, 2020-10-06 at 23:31 +0300, Rémi Denis-Courmont wrote:
> Le tiistaina 6. lokakuuta 2020, 8.58.01 EEST Lyndon Brown a écrit :
> > From: Lyndon Brown <jnqnfe at gmail.com>
> > Date: Mon, 5 Oct 2020 23:54:06 +0100
> > Subject: esout: refactor GetTmpFile()
> > 
> > to prep for use of config_GetTempPath() and avoid duplicated logic.
> > 
> > param names changed to ones that make better sense for clarity.
> > 
> > diff --git a/src/input/es_out_timeshift.c
> > b/src/input/es_out_timeshift.c
> > index 8ffa1ea400..3cc38e0445 100644
> > --- a/src/input/es_out_timeshift.c
> > +++ b/src/input/es_out_timeshift.c
> > @@ -1820,29 +1820,22 @@ static int CmdExecutePrivControl( es_out_t
> > *p_tsout,
> > ts_cmd_privcontrol_t *p_cmd }
> >  }
> > 
> > -static int GetTmpFile( char **filename, const char *dirname )
> > +static int GetTmpFile( char **path, const char *directory )
> >  {
> > -    if( dirname != NULL
> > -     && asprintf( filename, "%s"DIR_SEP PACKAGE_NAME"-
> > timeshift.XXXXXX",
> > -                  dirname ) >= 0 )
> > -    {
> > -        vlc_mkdir( dirname, 0700 );
> > -
> > -        int fd = vlc_mkstemp( *filename );
> > -        if( fd != -1 )
> > -            return fd;
> > -
> > -        free( *filename );
> > +    const char *dir_actual;
> > +    if (directory != NULL) {
> > +        dir_actual = directory;
> > +        vlc_mkdir(directory, 0700);
> >      }
> > -
> > -    *filename = strdup( DIR_SEP"tmp"DIR_SEP PACKAGE_NAME"-
> > timeshift.XXXXXX"
> > ); -    if( unlikely(*filename == NULL) )
> > -        return -1;
> > -
> > -    int fd = vlc_mkstemp( *filename );
> > -    if( fd != -1 )
> > -        return fd;
> > -
> > -    free( *filename );
> > -    return -1;
> > +    else
> > +        dir_actual = DIR_SEP"tmp";
> > +
> > +    int fd = -1;
> > +    const char *filetemplate = PACKAGE_NAME"-timeshift.XXXXXX";
> > +    if (asprintf(path, "%s"DIR_SEP"%s", dir_actual, filetemplate)
> > >= 0) {
> > +        fd = vlc_mkstemp(*path);
> > +        if (fd == -1)
> > +            free(*path);
> > +    }
> > +    return fd;
> >  }
> 
> How about *actually* factoring the admittedly repetitive code?
> 
> You can just call GetTempFile(filename, "/tmp"); and divide the
> length of the 
> function in half. This does not need any external change.

Yeah, that would make things even neater, not really 1/2 as much code,
only 3/4 to nitpick, but neater. As long as you don't mind the extra
function call and vlc_mkdir call overhead for the default-dir case.