From: Torsten Bögershausen <tboegi@web.de>
Date: Wed, 08 Jun 2016 08:17:30 GMT
Subject: Re: [PATCH 3/4] dir: introduce file_size() to check the size of file
Message-ID: <5757D49A.304@web.de>
In-Reply-To: <CAFZEwPPWMdOahMP4f0=Lm_n+ZmEtGHg=zHaZAyQjSf9oL8X2ew@mail.gmail.com>

On 06/08/2016 09:57 AM, Pranit Bauva wrote:
> 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 *);
>
So what I understand, you want something like this:

+ssize_t file_size_not_zero(const char *filename)
+{
+       struct stat st;
+       if (stat(filename, &st) < 0)
+               return -1;
+       return !!st.st_size);
+}