Re: [PATCH 3/4] dir: introduce file_size() to check the size of file

[Pranit Bauva <pranit.bauva@gmail.com>]

Hey Eric,

On Wed, Jun 8, 2016 at 1:07 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:
> On Tue, Jun 7, 2016 at 4:54 PM, Pranit Bauva <pranit.bauva@gmail.com> wrote:
>> dir: introduce file_size() to check the size of file
>>
>> At times we require to see if the file is empty and get the size of the
>> file. By using stat we can get the file size without actually having to
>> open the file to check for its contents.
>
> The sole caller of this function in patch 4/4 does so only to check if
> the file exists; it doesn't even care about the file's size, thus
> neither this function nor this patch seem justified and probably ought
> to be dropped unless some better and stronger justification can be
> shown.

Umm, actually there is a subtle difference between file_exists() and
file_size(). file_exists() *only* checks whether the file exists or
not while file_size() can also be used to check whether the file is
empty or not also see the implementation of both of them which shows
the difference. In fact it doesn't care at all about the file size.
Now there are a lot of instances in shell scripts where there are
quite some differences with -f and -s and some places *do care* about
this subtle difference. For eg. in bisect_reset() we test whether the
file .git/BISECT_START has some contents in it. But I guess I can add
some more justification in the commit message. What do you think?

>> Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com>
>> ---
>> diff --git a/dir.c b/dir.c
>> @@ -2036,6 +2036,14 @@ int file_exists(const char *f)
>> +ssize_t file_size(const char *filename)
>> +{
>> +       struct stat st;
>> +       if (stat(filename, &st) < 0)
>> +               return -1;
>> +       return xsize_t(st.st_size);
>> +}
>> +
>> diff --git a/dir.h b/dir.h
>> @@ -248,6 +248,13 @@ extern void clear_exclude_list(struct exclude_list *el);
>> +/*
>> + * Return the size of the file `filename`. It returns -1 if error
>> + * occurred, 0 if file is empty and a positive number denoting the size
>> + * of the file.
>> + */
>> +extern ssize_t file_size(const char *);

Regards,
Pranit Bauva