Re: [PATCH v2 3/6] wrapper: move is_empty_file() from builtin/am.c

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

 



Hey Junio,

On Wed, Jun 15, 2016 at 11:42 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Pranit Bauva <pranit.bauva@xxxxxxxxx> writes:
>
>> diff --git a/builtin/am.c b/builtin/am.c
>> index 3dfe70b..84f21d0 100644
>> --- a/builtin/am.c
>> +++ b/builtin/am.c
>> @@ -30,22 +30,6 @@
>>  #include "mailinfo.h"
>>
>>  /**
>> - * Returns 1 if the file is empty or does not exist, 0 otherwise.
>> - */
>> -static int is_empty_file(const char *filename)
>> -{
>> -     struct stat st;
>> -
>> -     if (stat(filename, &st) < 0) {
>> -             if (errno == ENOENT)
>> -                     return 1;
>> -             die_errno(_("could not stat %s"), filename);
>> -     }
>> -
>> -     return !st.st_size;
>> -}
>> -
>
> This is perfectly fine in the context of "git am", but as a public
> function that is called is_empty_file(), callers can come from two
> camps.  One is like the caller of this function in "am" where an
> empty and a missing file are treated equivalently.  The other would
> want to act differently.
>
> Renaming it "is-empty-or-missing" is necessary in order to make it
> clear that this helper function is not targetted for the latter
> callers.

Yes, its better to rename the function as is_empty_or_missing() . Thanks!

Regards,
Pranit Bauva
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]