Hey Torsten,
On Sun, Jun 12, 2016 at 4:14 PM, Torsten Bögershausen <tboegi@xxxxxx> wrote:
>>> 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);
>>> +}
>>
>> For the purpose of bisect_reset(), Yes. BTW a similar function exist
>> in builtin/am.c with the name is_empty_file(). But as Christian points
>> out file_size() could help to refactor other parts of code.
>>
>
> Please allow one or more late comments:
That's perfectly fine.
> If is_empty_file() does what you need, then it can be moved into wrapper.c
> and simply be re-used in your code.
Thanks for informing. I was unaware about the use of wrapper.c
> If you want to introduce a new function, that can be used for other refactoring,
> then the whole thing would ideally go into a single commit,
> or into a single series.
> That may probably be out of the scope for your current efforts ?
On re-thinking, I think introducing file_size() is out of the scope
for the current efforts and I will stick to is_empty_file(). Will move
it to wrapper.c and then use it in my code. I am not sure but I think
a few other parts could also use is_empty_file(). I will check on that
probably after GSoC as a cleanup.
> What really makes me concern is the mixture of signed - and unsigned:
> ssize_t file_size(const char *filename)
> +{
> + struct stat st;
> + if (stat(filename, &st) < 0)
> + return -1;
> + return xsize_t(st.st_size);
> +}
>
> To my understanding a file size is either 0, or a positive integer.
> Returning -1 is of course impossible with a positive integer.
True.
> So either the function is changed like this:
>
> int file_size(const char *filename, size_t *len)
> +{
> + struct stat st;
> + if (stat(filename, &st) < 0)
> + return -1;
> + *len = xsize_t(st.st_size);
> + return 0;
> +}
>
> Or, if that works for you:
>
> size_t file_size(const char *filename)
> +{
> + struct stat st;
> + if (stat(filename, &st) < 0)
> + return 0;
> + return xsize_t(st.st_size);
> +}
>
> Or, more git-ish:
>
> size_t file_size(const char *filename)
> +{
> + struct stat st;
> + if (stat(filename, &st))
> + return 0;
> + return xsize_t(st.st_size);
> +}
>
> (And then builtin/am.c can be changed to use the new function.
I think I will just skip file_size() for now.
Thanks for your comments!
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