Thread (33 messages) flat view 33 messages, 4 authors, 2017-08-18

Re: [PATCH v3 2/4] commit: replace the raw buffer with strbuf in read_graft_line

From: Patryk Obara <hidden>
Date: 2017-08-18 10:13:14

Jeff King [off-list ref] wrote:
AFAICT this is only here to avoid having to s/buf/line->buf/ in the rest
of the function. But I think we should just make that change (you
already did in some of the spots). And IMHO we should do the same for
line->len. When there are two names for the same value, it increases the
chances of a bug where the two end up diverging.
My motivation was rather to keep patch(es) as small as possible because every
line using buf will be replaced in a later patch in series. But it will make
commit better (it will stand on its own), so why not to do it? :)
(…) I think short-circuiting like:

  if (!line->len || line->buf[0] == '#')

is better (I also think "!" instead of "== 0" is our usual style, but
that's much less important).
Ah, I only replaced comparison to NULL terminator with length check because
I thought it better shows intention of the code and I didn't notice, that
reversing order will result in better code overall.

I will include both changes in v4.

-- 
| ← Ceci n'est pas une pipe
Patryk Obara
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help