Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> 于2021年7月22日周四 下午4:10写道：
>
>
> On Thu, Jul 22 2021, ZheNing Hu via GitGitGadget wrote:
>
> > [..] add a new member in
> > `struct atom_value`: `s_size`, which can record raw object size, it
> > can help us add raw object data to the buffer or compare two buffers
> > which contain raw object data.
>
> I didn't look in detail, just one nit/comment, no need to re-roll for this:
>
>
> > struct atom_value {
> > const char *s;
> > + size_t s_size;
> > int (*handler)(struct atom_value *atomv, struct ref_formatting_state *state,
> > struct strbuf *err);
> > uintmax_t value; /* used for sorting when not FIELD_STR */
> > struct used_atom *atom;
> > };
> >
> > +#define ATOM_VALUE_S_SIZE_INIT (-1)
> > +
>
> This and then later doing (this appears to be the only initialization?):
Yes, "s_size = -1” is the only initialization.
>
> > if (format->need_color_reset_at_eol) {
> > struct atom_value resetv;
> > + resetv.s_size = ATOM_VALUE_S_SIZE_INIT;
> > resetv.s = GIT_COLOR_RESET;
> > if (append_atom(&resetv, &state, error_buf)) {
> > pop_stack_element(&state.stack);
>
> Is a rather unusual pattern, more typical would be:
>
> struct atom_value { .... }
> #define ATOM_VALUE_INIT { \
> .s_size = -1, \
> }
>
> Then:
>
> struct atom_value resetv = ATOM_VALUE_INIT;
>
> You could keep that "#define ATOM_VALUE_S_SIZE_INIT (-1)" to check the
> init value, personally I think it's just fine to hardcode the -1 literal
> and do "size < 0" checks, which we us as convention in a lot of other
> places for "not initialized".
Maybe you are right, hard-coding "-1" is ok.
Thanks.
--
ZheNing Hu
