From ef507ad50a0933581f39a0bb86dd85fce9d8f7bc Mon Sep 17 00:00:00 2001 From: wm4 Date: Thu, 13 Jun 2019 18:23:05 +0200 Subject: [PATCH] osdep: add mkostemps() emulation Supposed to follow the standard function. The standard function is not standard, but a GNU extension. Adding some ifdef mess is pointless too - it has no advantages other than having a mess, and not spotting implementation bugs in the emulation due to running it only on "obscure" platforms (like Windows, so most computers actually, except the developer's platform). There is mkstemp(), which at least is in POSIX 2008. But it's 100% useless, except in some obscure cases: it doesn't set O_CLOEXEC, nor can you pass it to it. Without O_CLOEXEC, we'd leak the temporary file to all child processes. (The fact that the file, which is expected to reach double or tripple digit GB sizes, will be deleted only once all processes unreference the FD, makes this sort of a big deal. You could ftruncate() it, but that doesn't fix all the other problems.) Why did POSIX standardize mkstemp() and O_CLOEXEC apparently at the same time, but provided no way to pass O_CLOEXEC to mkstemp()? With the introduction of O_CLOEXEC, they acknowledged that there's a need to atomically set the FD_CLOEXEC flag when creating file descriptors. (FD_CLOEXEC was standard before that, but setting it with fcntl() is racy.) You're much more likely to need a temp file that is CLOEXEC rather than the opposite, and even if they were somehow opposed to CLOEXEC by default (such as for compat. reasons), surely POSIX could have standardized mkostemp() too or instead. And then there's the fact that this whole O_CLOEXEC mess is stupid. Surely there would have been a better way to handle this, instead of requiring adding O_CLOEXEC to almost ALL instances of open() in all code that has been written ever. The justification for this is that the historic default was wrong, and you can't change it (e.g. this won't work: changing the behavior of exec() and not inherit the FD to the child process, unless a hypothetical O_KEEP_EXEC flag is set). But on the other hand, surely you could have introduced an exec() variant which does close all FDs, except a whitelist of FDs passed to it. Let's call it execve2(). In fact, I'm going to argue that exec() call sites are the most aware of whether (and which) FDs to inherit. Some programs even tried to explicitly iterate over all opened FDs and explicitly close "unwanted" FDs (which of course was problematic for other reasons), and such an execve2() call would have been the ideal solution. Maybe this proposed solution would have had problems too. But surely revisiting and reviewing every exec*() call would have been simpler than reviewing every open() call. And more importantly, having to extend every damn library function that either calls open() or creates FDs in some other way, like mkstemp(). What argument are there going to be against this? That there will be library code that can't keep working correctly with processes that use the "old" exec? Well, what about all my legacy library code that uses open() incorrectly, and that will break no matter what? Well, I'm not going to claim that I can come up with better solutions than POSIX (generally or in this case), but this situation is ABSOLUTELY ATROCIOUS. It makes win32 programming look attractive compared to POSIX, that standard pandering to dead people from the past. (Note: not trying to insult dead people.) I'm not sure what POSIX is even doing. Anything useful? Doesn't look like it to me. Are they paid? Why? They didn't even fix the locale mess, nor do they intend to. I bet they're proud of discussing compatibility to 70ies code day in and day out iwtohut ever producing anything useful. What a load of crap. They seriously got to do better than this. Oh, and my wrapper is probably buggy. Fortunately that doesn't matter. Also I'm dumping this into io.h. Originally, io.h was just supposed to replace broken implementation of standard functions by MinGW (and then by Android), but whatever, just give a dumping ground for shit code. --- osdep/io.c | 36 ++++++++++++++++++++++++++++++++++-- osdep/io.h | 2 ++ 2 files changed, 36 insertions(+), 2 deletions(-) diff --git a/osdep/io.c b/osdep/io.c index 3b2061e55b..f45b93a333 100644 --- a/osdep/io.c +++ b/osdep/io.c @@ -19,9 +19,15 @@ * License along with mpv. If not, see . */ -#include -#include #include +#include +#include +#include +#include +#include +#include +#include +#include #include "mpv_talloc.h" @@ -814,3 +820,29 @@ void freelocale(locale_t locobj) } #endif // __MINGW32__ + +int mp_mkostemps(char *template, int suffixlen, int flags) +{ + size_t len = strlen(template); + char *t = len >= 6 + suffixlen ? &template[len - (6 + suffixlen)] : NULL; + if (!t || strncmp(t, "XXXXXX", 6) != 0) { + errno = EINVAL; + return -1; + } + + for (size_t fuckshit = 0; fuckshit < UINT32_MAX; fuckshit++) { + // Using a random value may make it require fewer iterations (even if + // not truly random; just a counter would be sufficient). + size_t fuckmess = rand(); + char crap[7] = ""; + snprintf(crap, sizeof(crap), "%06zx", fuckmess); + memcpy(t, crap, 6); + + int res = open(template, O_RDWR | O_CREAT | O_EXCL | flags, 0600); + if (res >= 0 || errno != EEXIST) + return res; + } + + errno = EEXIST; + return -1; +} diff --git a/osdep/io.h b/osdep/io.h index 2f04337288..131f3e2fee 100644 --- a/osdep/io.h +++ b/osdep/io.h @@ -207,4 +207,6 @@ void freelocale(locale_t); #endif /* __MINGW32__ */ +int mp_mkostemps(char *template, int suffixlen, int flags); + #endif