On Thu, Mar 17, 2011 at 14:21, FX <fxcoud...@gmail.com> wrote:
> Thanks for the review!
>
>> - Use the type size_t for tempdirlen as that is the return type of
>> strlen() and argument type for get_mem().
>>
>> - You can use a const size_t variable for the length of the string
>> "slash" rather than calling strlen() in the do-while loop.
>
> Both OK.
>
>> - Don't set errno as we anyway loop until we successfully open a file
>> with O_CREAT|O_EXCL.
>
> No, if I don't set errno, the continue statement will not iteration the loop,
> as the condition (fd == -1 && errno == EEXIST) is false. Unless I don't
> understand what you mean.
I was thinking that maybe it's a bit bad style to set errno after
getting an error return from a function that may itself have set errno
(depending on which vendor manpage one reads, mktemp() may or may not
set some errno value, so this is a bit poorly specified, but I
digress), thus overwriting to real cause of the error with an error
code that may or may not have anything to do with the actual failure.
That is, maybe something like
fd = -1;
do
{
// Create template
if (!mktemp(...)) continue;
fd = open(...);
if (fd == -1 && errno != EEXIST) break;
} while (fd == -1)
is a bit clearer, but ultimately I suppose the end result is the same.
So on second thought I guess your original code is Ok too, if you
prefer that.
--
Janne Blomqvist
