-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 According to Bruno Haible on 10/5/2006 6:30 AM: > > It's the latter: You can choose arbitrary file names in the temp_dir. For > example, you could safely unzip a file in the temp_dir (assuming none of > the file names contain a ".." component :-)).
Hmm. The gnulib mkdtemp module does not take umask into account. Neither does the mkdtemp variant of lib/tempname.c. Shouldn't these modules be guaranteeing that the directory created has full user permissions, in spite of the current umask? Or is it the caller's responsibility (such as is documented by the mkdir-p module) to ensure that umask is 0 at the time of calling mkdtemp/create_temp_dir/...? > >> Also, if I keep M4's usage of FILE*, then I imagine I should ensure that >> each >> of the file objects is closed before the temp_dir cleanup functions get >> invoked. > > Ouch, indeed. It was reported that on Woe32 you cannot delete an open file. > The Woe32 function CreateFile accepts a FILE_SHARE_DELETE argument which would > overcome this limitation, but neither the MSVC open() nor the Cygwin open() > function provide a way to set this flag in the CreateFile call. Actually, cygwin allows unlinking a file while it is open, unlike native Windows (as it proves very useful for porting programs with typical mkstemp/unlink usage patterns, in spite of the fact that POSIX allows unlink to fail on an open file). However, cygwin currently inherits Windows limitations of not deleting a directory when any process has that directory as its pwd, or when that directory contains an open fd, whether or not that fd maps to an unlinked file (as evidenced by the patch I made to gnulib-tool last month). Interix seems to have come up with some workaround for this, and maybe a future version of cygwin will be able to borrow Interix's solution; but I don't use Interix enough to be familiar with it. But the point remains - if a fatal signal arrives, the current implementation of clean-temp blindly calls unlink and rmdir on files that may still be locked, and will fail to clean up everything on Windows if the implementation still has any temp files open. > > What I propose is to add a registry of open file descriptors to the clean-temp > module, and wrappers temp_open, temp_close, temp_fopen, temp_fclose, > temp_fwriteerror of open, close, fopen, fclose, fwriterror, so that this > code gets simplified to fwriterror is still controversial for the reasons Paul mentioned - it writes to the file descriptor in some situations that close-stream can currently avoid. The rest of those wrappers sound reasonable. Speaking of which, now that coreutils 6.3 is out, we should go ahead and patch the closeout module to check for write errors on stderr. That proposed change was not as controversial as the changes to close-stream. > > register_temp_file (tmpdir, java_file_name); > java_file = temp_fopen (java_file_name, "w"); Would it make more sense for this to resemble openat semantics (passing filenames relative to the reference point of the tmpdir), and look something like: register_temp_file (tmpdir, java_file_base_name); java_file = temp_fopen (tmpdir, java_file_base_name); with the implied semantics change to register_temp_file that if it is passed a tmpdir-relative file name, it can construct the appropriate absolute name rather than requiring users to construct the absolute name? > if (java_file == NULL) > { > error (0, errno, _("failed to create \"%s\""), java_file_name); > unregister_temp_file (tmpdir, java_file_name); > goto quit; > } > write_java_code (java_file); > if (temp_fwriteerror (java_file)) > { > error (0, errno, _("error while writing \"%s\" file"), java_file_name); > goto quit; > } > > Would this approach be fine for m4? Yes, having a way to register open fds/open FILES alongside the filenames to delete would be helpful. > >> That makes it sound like I would need to register another fatal >> handler that gets invoked before the temp_dir fatal_handler (does >> at_fatal_handler guarantee LIFO execution of fatal handlers?). > > There is no such guarantee. It is too dangerous to rely on this. > Actually fatal-signal currently installs its handlers overwriting the > previously installed signal handler (if any). I agree that once you use at_fatal_signal, your program cannot manipulate fatal signal handlers any more, because you have handed that over to the fatal-signal module. My question was more along the lines of if you use at_fatal_signal multiple times, are all of its registrants executed in LIFO order? If so, then I can register a fatal signal handler after calling create_temp_dir to do any cleanup that I know is needed before temp_dir's cleanup. >> It would be >> nice if fatal-signal could be augmented with a way to register a handler >> function that takes a void* argument along with the argument to invoke it >> with >> (similar to on_exit semantics), at which point I could repeatedly register >> the >> same handler but with a different FILE* object and let fatal-signal manage >> the >> list of registered FILE* to close. > > When you do this, you will also need to unregister the (handler function, > argument) pairs that you have registered. (Otherwise in a long-running > program such pairs accumulate without bounds.) And now you have a race > condition: If you do > unregister_pair (handler, fp); > fclose (fp); > there is a small chance that the program gets SIGTERM between the two > statements, and then the temporary file cannot be removed. If, on the > other hand, you do > fclose (fp); > unregister_pair (handler, fp); > then there is a small chance that a SIGTERM arrives between the two > statements, and the handler tries to access the already freed memory > at fp, and crashes. > To avoid this race condition, you need block/unblock_fatal_signals(). Point taken. So for now, don't worry about implementing an on_exit style fatal signal handler interface. - -- Life is short - so eat dessert first! Eric Blake [EMAIL PROTECTED] -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.2.1 (Cygwin) Comment: Public key at home.comcast.net/~ericblake/eblake.gpg Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org iD8DBQFFJQSS84KuGfSFAYARAurDAKDHmCKzJLHYFwMw3nTnZGvmqQG0/wCffCMv 1I+Zm6es9Qw0HAWiMxCa49M= =cHW0 -----END PGP SIGNATURE-----