[vlc-devel] [PATCH v2 6/13] tests: make use of config_GetTempPath()

Lyndon Brown jnqnfe at gmail.com
Tue Oct 6 20:57:16 CEST 2020


On Tue, 2020-10-06 at 09:07 +0200, Steve Lhomme wrote:
> On 2020-10-06 7:59, Lyndon Brown wrote:
> >   #ifndef TEST_NET
> > -    char psz_tmp_path[] = "/tmp/libvlc_XXXXXX";
> >       char *psz_url;
> > -    int i_tmp_fd;
> >   
> >       test_log( "Generating random file...\n" );
> > -    i_tmp_fd = vlc_mkstemp( psz_tmp_path );
> > +
> > +    char *psz_tmp_dir = config_GetTempPath();
> > +    assert(psz_tmp_dir != NULL);
> > +
> > +    char *psz_tmp_path;
> > +    const char *psz_tmp_filetemplate = "libvlc_XXXXXX";
> > +    assert(asprintf(&psz_tmp_path, "%s"DIR_SEP"%s", psz_tmp_dir,
> > psz_tmp_filetemplate) >= 0);
> > +    free(psz_tmp_dir);
> > +
> > +    int i_tmp_fd = vlc_mkstemp(psz_tmp_path);
> 
> nit picking: For better readability of the patch it's cleaner to
> keep 
> lines from the original when possible. In this case the 'i_tmp_fd' 
> declaration and setting the value also don't have to change.
> 
> Then the patch will clearly show that the modified line is the one 
> setting psz_tmp_path.

Yeah, I try to keep such things to a minimum, but it's nice to tidy
when the opportunity arises. In this case I felt that it did not
negatively impact readability enough to not do so, but I can adjust if
really wanted.

Then again, perhaps it would be better as part of the later commits
using the new vlc_MakeTmpFile() helper, since that changes those lines
anyway. Yeah, I'll do that...



More information about the vlc-devel mailing list