On Thursday, December 02, 2004 2:43 AM, Tomasz Kojm wrote:
On Wed, 1 Dec 2004 18:32:33 -0800
"Mark Pizzolato" <[EMAIL PROTECTED]> wrote:

> > Every normal POSIX compliant platform provides reentrant and thread
> > safe tmpfile().
>
> Solaris man pages for tmpnam(), and tempnam() and tmpfile() suggest
> that tmpfile() uses tmpnam() and that tmpnam() is not threadsafe.

No, it doesn't. It just says that "The tmpnam() function is unsafe  in
multithreaded  applications. The  tempnam()  function  is  safe in
multithreaded applications and should be used instead." No word about
tmpfile().

Also `man -s5 attributes` doesn't list tmpfile() as a MT-unsafe
function.

That is true, however the tmpfile() page specifically mentions tmpnam() (not tempnam())AND unlike other functions which are MT-Level "Safe", makes no mention of MT-Level-ness. Given that tmpnam() is explicitly unsafe and the direct dependent reference to tmpnam() from tmpfile() and Lacking access to the source, a reasonable conclusion is that tmpfile() like tmpnam() is MT unsafe.

> exists a real likelyhood that collisions will occur on systems which
> have relatively low resolution updates to the microsecond return from
> gettimeofday().

To protect against potential collisions it uses a last result of
cli_md5buff as a salt.

Ahh. Now I see. Given this approach, what is being protected by the mutex in cli_gentemp() ?

By the way, it would seem that cli_rndnum() should return a result modulo (max+1) instead of max if max truly represents tht maximum value to be returned. It seems from the only usage of cli_rndnum() that the caller is presuming maximum value.

OK, so there is not an advantage to the internal serialization of the code I suggested. However, just like pulling in the getenv() stuff from all of the callers of cli_gentemp, the added APIs simplify the calling code, whch is a good number of places. An example is each place that calls cli_gentemp() to generate a directory name, and then calls mkdir(). Like:

tempname = cli_gentemp(NULL);
if(mkdir(tempname, 0700)) {
cli_dbgmsg("ScanHTML - > Can't create temporary directory %s\n", tempname);
return CL_ETMPDIR;
}

is replaced by:
if ((tempname = cli_gentempdir(NULL)) == NULL) {
cli_dbgmsg("ScanHTML - > Can't create temporary directory\n");
return CL_ETMPDIR;
}

Note, that the original code is already broken (it needs to be changed to the NULL: return which cli_gentemp can return).

Attached is a patch which removes the remaining tmpfile() occurrances in libclamav. While doing that it also fixes a few open file leaks in error paths.


Attachment: 20041130.patch.new2
Description: Binary data

_______________________________________________
http://lists.clamav.net/cgi-bin/mailman/listinfo/clamav-devel

Reply via email to