Re: [PATCH 09/17] lib: add fileutils function collection

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Mar 5, 2012 at 22:51, Davidlohr Bueso <dave@xxxxxxx> wrote:
> On Mon, 2012-03-05 at 20:38 +0100, Sami Kerola wrote:
>>  create mode 100644 include/fileutils.h
>>  create mode 100644 lib/fileutils.c
>
> Doesn't really make much sense creating two files for just one
> function.  Couldn't xmkstemp() so somewhere else?  You might argue
> that xgetpass also does that, I just think it's an overkill...

Hi Davidlohr,

You are right, at least partially.  The reason why I added the files
is simply that non of the existing files felt correct to add this
function.  IMHO xgetpass.c would be more strange file to have
xmkstemp() than fileutils.c to contain xgetpass().

>> +/* Create open temporary file in safe way.  Please notice that the
>> + * file permissions are -rw------- by default. */
>> +FILE *xmkstemp(char **tmpname)
>
> Returning the file descriptor seems a more flexible interface
> instead of the FILE's representation; the user can always use
> fdopen on his own.

If I need temporary file I rather have stream than file descriptor.
Could you give example why flexibility is better than completeness.
With completeness I mean service that the function provides.  If
after every single occurrence of xmkstemp() there would be fdopen() I
would argue adding flexibility did not do anything good.

>> +     fd = mkstemp(localtmp);
>> +     umask(old_mode);
>> +     if (fd == -1)
>> +             goto err;
>
> the file isn't open on failure, just return NULL.

You are right. Fixed in my git.

Thank you for feedback.

-- 
   Sami Kerola
   http://www.iki.fi/kerolasa/
--
To unsubscribe from this list: send the line "unsubscribe util-linux" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Netdev]     [Ethernet Bridging]     [Linux Wireless]     [Kernel Newbies]     [Security]     [Linux for Hams]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux Admin]     [Samba]

  Powered by Linux