Re: [PATCH] Coverity issue 740457

On 24/01/2013 4:30 a.m., Kinkie wrote:
>> This seems to be droppign the small mkstemp optimization makign use of its
>> FD return result as the already open file.
>>
>> I think we should be calling umask() before before the whole #if segment and
>> reset it on each of "return" as well as on successful completion.
> Hi,
> v2 of the patch, which does as you suggest (and removes a few useless
> empty lines).
>
>
> --
> /kinkie

That looks much better.

* the string created by tempnam() apparently needs to be xfree()'d when
done.

* please add a comment in the #else condition noting the tempnam() is
officially obsolete from POSIX-2008.1.
   (in a few more years we should drop the #else code entirely.)

* please also add a note that tmpfile() is not an option since we desire
the /tmp file to stick around after process exit for administrators use.

As a followup change we need to consider that this whole process leaves
a /tmp file sitting around from every crash, which could over time build
up. I'm in two minds about how desirable this behaviour is.
  + we could check for the files on startup and do something when Squid
is re-started after a crash.
  + administrator can manually find the file even if the mail attempt
failed (known to happen).

  - the files build up over time and could be a big waste of disk space.

Amos
Received on Wed Jan 23 2013 - 23:32:27 MST