From: Christian Couder <christian.couder@gmail.com>
Date: Wed, 08 Jun 2016 10:03:26 GMT
Subject: Re: [PATCH 3/4] dir: introduce file_size() to check the size of file
Message-ID: <CAP8UFD0PBDnn0MV0+vUJkS-Smazi6x0cfq4EJot8fDpG4RqH1g@mail.gmail.com>
In-Reply-To: <CAPig+cR8+ekOv3xuj1Nn9ftA5smHG78wdFp1Xnfz_qyrstKRqw@mail.gmail.com>

On Wed, Jun 8, 2016 at 10:13 AM, Eric Sunshine <sunshine@sunshineco.com> wrote:
>
> I think this would be clearer if you instead added a function to
> bisect--helper.c which operates at a semantically higher level than
> what you have here (and drop this file_size() function). Specifically,
> add a function which tells you precisely what you want to know:
> whether the file exists and has non-zero size. For instance, in
> bisect--helper.c:
>
>     static int file_empty_or_missing(const char *path)
>     {
>         struct stat st;
>         return stat(...) < 0 || st.st_size == 0;
>     }
>
> Or, if you have more cases where you want to know that it exists with
> non-zero size, then name it file_non_empty() and reverse the sense of
> the return value.

After a quick grep it seemed to me that there are a few places were we
stat a file just to get its size, so I suggested the file_size()
because it could help refactor other parts of the code.