[vlc-devel] [PATCH] record: use vlc_mkstemp instead of tempnam

[Lyndon Brown]

On Mon, 2020-09-28 at 07:48 +0200, Steve Lhomme wrote:
> On 2020-09-27 22:30, Lyndon Brown wrote:  
> > +            psz_file = strdup( DIR_SEP"tmp"DIR_SEP"vlc-
> > recording.XXXXXX" );
> 
> I'm pretty sure /tmp doesn't exist on Windows. And it will
> definitely 
> not be cleaned by the system when the user tells the OS to clean the 
> temporary files.

Hi :)

FWIW, just to nitpick, on Windows it wouldn't be /tmp, we'd be talking
about "\tmp\foo" due to DIR_SEP being a backslash on Windows, which
would be interpreted as being <current-device>\tmp\foo.

Yes indeed there's a problem with respect to Windows; This wasn't a
failure to recognise the problem there, I've simply completely
forgotten that when I originally made the patch last year that I
intended to raise this question of what we should do here wrt. Windows
when I eventually submitted it, which is coming back to me now.
Apologies for forgetting and thanks for catching it.

The behaviour of the tempnam() function call being replaced, per the
man page, is that it follows a set of defined rules to determine where
to create the file. The mkstemp() function and friends don't do this I
don't think (the man page is not perfectly clear), and certainly the
implementation of vlc_mkstemp() in src/text/filesystem.c doesn't do any
such thing.

I copied use of `DIR_SEP "tmp" DIR_SEP` from other existing use of the
function in the codebase, though it seemed wrong in relation to
Windows, hence having intended to ask. We have three unit tests with
hard coded "/tmp/foo" strings, and this same `DIR_SEP "tmp" DIR_SEP`
thing being done in the GetTmpFile() function of es_out_timeshift.c
when it's not given a path. (There are also hard coded "/tmp/foo" paths
in vlc_memfd() for posix/linux, but at least that's not so
problematic).

So what is up with these existing users of vlc_mkstemp() wrt. Windows?
Why are these tests hard coding *nix paths? Why does GetTmpFile() use a
*nix path if not given a directory path?

And how do we approach using it correctly as a proper replacement for
tempnam() in record and vorepository?

It would appear to me that we need to dynamically define the correct
standard system temp path to use, either in a hard coded way or via
some header, and then use that for our vlc_mkstemp() calls? Right?