#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
