Hi! I'm interested in taking this one (trying hacktoberfest for the first time)

In almost all case, it seems this function is called to instantiate an fstream, so it would seem to make sense to return an opened fstream directly. In a few other cases however, it seems only the name is needed, and the file should remain closed.

There could either be two functions, one returning the name, to maintain 'legacy' behaviour, and the other returning a fstream.
Alternatively, std::fstream could be wrapped to also store the filename.

@wwmayer could you weigh-in on this?

Hey @berniev could you take a stab at this?

@luzpaz Thank you for your thinking of me, but to be honest this question is still outside the realm of my knowledge and experience.

@nebularnoise this maybe too ambitious. We need @wwmayer to weigh-in. But in the meantime, feel free to choose another issue?

The function getTempFileName() is supposed to return the name of a tmp. file. Therefore it uses some OS-specific functions that already create a handle of a tmp. file and release it immediately (Windows: GetTempFileName / DeleteFile and other OS mkstemp / unlink) just to get this name.

The problem is that when trying to open the file for writing there is no guarantee that in the meantime any other process already has access on this file. The consequence may be that the access will be denied or even worse the file will be (partially) overridden.

The better way would be to return a class object (e.g. TemporaryFile) where its constructor creates the file and the destructor releases it.

I was looking through the codebase to see where this function is used, and it looks like there are a few places where something is appended to the temporary filename. I was wondering what the best way to handle that would be? Here's an example:

@CalebHeydon
In terms of the exposed API, maybe adding a "suffix" argument would work?
On top of that, as @wwmayer suggested, if the function returns a TemporaryFile object, it would prevent future "abuses" of this function call which modify the filename.

In terms of implementation, mkstemps supports suffixes out of the box. For Windows, I'm not aware of a system call which does, but I'm no expert. In any case, if we're to return a file handle, it will be necessary to check whether the file creation is successful, so we might as well move this "append" within the implementation, and try to create the file until successful.

Adding a suffix argument is a good idea. What would be the benefit of creating a TemporaryFile wrapper over returning an fstream? File deletion?

Yes, I assume in many cases, the file is ultimately deleted. For that, an fstream is inadequate.

The original comment suggests one workaround, using a std::FILE* directly.

Wrapping a std::FILE* into a TemporaryFile would provide a much friendlier interface, with unique_ptr-like "lifetime management", and prevent reintroduction of similar race condition issues.

I not a C++ expert. How would you implement the TemporaryFile lifetime management? Why not just have the function return a unique pointer?

unique_ptr applies the RAII philosophy to memory allocation, this class would do the same but for files on the filesystem.

I've been playing with the idea on Compiler Explorer, here's what I've got so far
https://godbolt.org/z/9cGxWoW98