Re: [PATCH/WIP v3 06/31] am: detect mbox patches

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

 



On Fri, Jun 19, 2015 at 5:02 AM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Paul Tan <pyokagan@xxxxxxxxx> writes:
>
>> +static int is_email(const char *filename)
>> +{
>> +     struct strbuf sb = STRBUF_INIT;
>> +     FILE *fp = xfopen(filename, "r");
>> +     int ret = 1;
>> +
>> +     while (!strbuf_getline(&sb, fp, '\n')) {
>> +             const char *x;
>> +
>> +             strbuf_rtrim(&sb);
>
> Is this a good thing?  strbuf_getline() already has stripped the LF
> at the end, so you'd be treating a line with only whitespaces as if
> it is a truly empty line.
>
> I know the series is about literal translation and the script may
> lose the distinction between the two, but I do not think you need
> (or want) to be literally same for things like this.
>
> Same comment applies to other uses of "trim" in this patch.

No, the uses of strbuf_trim() are not good at all. Will remove them.

Thanks,
Paul
--
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]