Hi Phillip, On Mon, Apr 06, 2020 at 03:42:59PM +0100, Phillip Wood wrote: > Hi Denton > > On 06/04/2020 15:03, Phillip Wood wrote: > > Hi Denton > > > > On 04/04/2020 02:11, Denton Liu wrote: > > > In the original read_oneliner() logic, we duplicated the logic for > > > strbuf_trim_trailing_newline() with one exception: instead of preventing > > > buffer accesses below index 0, it would prevent buffer accesses below > > > index `orig_len`. Although this is correct, it isn't worth having the > > > duplicated logic around. > > > > > > Reset `buf` before using it then replace the trimming logic with > > > strbuf_trim(). > > > > I should have picked up on this before but this changes the semantics of > > the function as it strips all whitespace from the start and end of the > > strbuf. Above you talked about using strbuf_trim_trailing_newline() > > instead which would not change the semantics of this function. You could > > test to see if we've read anything and only call > > strbuf_trim_trailing_newline() in that case without messing with > > strbuf_reset(). (there is a corner case where if the buffer ends with > > '\r' when the function is called and it reads a single '\n' then the > > '\r' would be stripped as well but I think that is unlikely to happen in > > the wild) > > I'd also be fine with dropping this patch and leaving the trimming code as > is Yeah, I'll just drop this patch. I don't think it's worth holding up the rest of the series on this small detail. I'll probably try to resurrect this at a later time. Thanks, Denton > Best Wishes > > Phillip
