Thread (107 messages) 107 messages, 5 authors, 2021-03-11

Re: [PATCH v5 31/39] bundle: add new version for use with SHA-256

From: brian m. carlson <hidden>
Date: 2020-07-29 22:29:07

On 2020-07-29 at 02:53:47, Eric Sunshine wrote:
Moving the strbuf_rtrim() earlier in the loop, as suggested in my
previous review, made the diff a lot noisier, uglier, and more
difficult to read, however, the code itself ends up being easier to
reason about than in the previous round. Good.
Yeah, I noticed that.  The code is much nicer, so thank you for that
suggestion.
By the way (I didn't think of this in my previous review), but
wouldn't it be better for this:

    if (header->version == 3 && *buf.buf == '@') {

to instead be written as:

    if (header->version >= 3 && *buf.buf == '@') {

to future-proof it since versions beyond 3 will also almost certainly
support "@foo" capabilities?
I think this will need to be dealt with by the author of bundle v4,
since currently bundle v4 will error out before we get here.  I don't
want to make too many assumptions about a future format.
-- 
brian m. carlson: Houston, Texas, US

Attachments

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help