[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