#3638: Compilation errors for 1.6 -----------------------+---------------------- Reporter: grarpamp | Owner: mutt-dev Type: defect | Status: new Priority: major | Milestone: 1.6 Component: mutt | Version: 1.5.21 Resolution: | Keywords: -----------------------+----------------------
Comment (by invalid@…): {{{ On Thu, Apr 18, 2013 at 10:36:10AM -0000, Mutt wrote: It's not that mkstemp() isn't safe if you need to add a suffix (e.g. a file extension); it's that it won't do that. You have to take the filename you get back and add the suffix yourself, and THEN open the file properly, expecting that it could fail. It's not a question of safety of mkstemp(); it's a question of the standard library routines lacking imagination in how people might want their temporary files named[1]. Oh, and then, you need to remember to close the file that mkstemp() opened for you, that you didn't actually want. The bottom line is that without implementing a very complicated mutt-specific temp file routine which generates its own random filenames of somewhat arbitrary construction, the way Mutt uses mktemp() is about as optimal as one can expect to get in their temp file maker thingy. [END RELEVANT COMMENT] -=-=-=-=-=- [BEGIN RELATED DIATRIBE] [1] This isn't so unreasonable, though it turns out to be non-ideal. Temp files are meant to be transient, and as such their names should usually not be interesting. However in the case of, for example, MIME file attachments whose readers insist that the files they read be named a particular way, despite the file being perfectly valid input for that reader regardless of its name, it's a bit more complicated than that, and the library routines provided by the standard do not suffice. This is where mkstemps() and its friends come in... only there are two problems with it (IMO). The first is that it is not part of the standard, so your system may not have it. That makes it a tough sell in portable code. The second is, IMO, that the interface is brain-dead. The variable part of the template is always the string "XXXXXX" and no other, so this should be assumed. The interface can then become: int mkstemps(const char *prefix, const char *suffix, char *filename); This obviates the need to count the characters in the suffix (or in any part of the file name), eliminating an opportunity for a silly programming error. And it lets you pass in string constants (or macros that expand to string contsants); often the prefix and suffix would be constant, e.g. something like "mutt_attachment" and ".html". By contrast, with the current implementation of mkstemps(), you have to do all of the following: - assemble the prefix, "XXXXXX", and the suffix into a char array - count the suffix characters (perhaps by calling strlen()) - make sure the array you're putting this all in was the right size and make sure your code that assembles the string does not overrun the array The last one could be done a couple ways, depending on whether the suffix and prefix are known string constants. You might count the characters manually. You might call strlen() a couple of times. You might pre-allocate a string array of the correct size, if they are both constant. You may dynamically allocate the space, if they are not both constant (especially if memory pressure is an issue). Or you might allocate an array thought to be sufficiently large, and then make sure it's not overrun. This is a lot to get wrong, and it's completely pointless: The library function can do all of this for you. Then, all you have to do is to remember to call free(). Granted that's a common programming error, but it's just one, and chances are that with the current interface, you were going to do that anyway... The point is, even if your system has mkstemps(), you're probably better off using mktemp() the way Mutt uses it instead. You just have to make sure that the file does not already exist when you go to create it, and that you create the file 0600 so that evildoers can't write their exploit over your data... and Mutt does that. With older versions of glibc, mk*tmp*() opened the file with 0666, so you had to also remember to set your umask to prevent people from executing an exploit on your tempfile. }}} -- Ticket URL: </ticket/3638#comment:7> Mutt <http://www.mutt.org/> The Mutt mail user agent