Junio C Hamano <gitster@xxxxxxxxx> 于2021年7月24日周六 上午12:38写道：
>
> "ZheNing Hu via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:
>
> > 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_INIT { \
> > + .s_size = -1 \
> > +}
>
> This will get turned into size_t that is unsigned, and comparisons
> like this one and ...
>
> > case QUOTE_NONE:
> > - strbuf_addstr(s, str);
> > + if (len != -1)
> > + strbuf_add(s, str, len);
> > + else
> > + strbuf_addstr(s, str);
>
> this one ...
>
> > + if (v->s_size != -1)
> > + strbuf_add(&state->stack->output, v->s, v->s_size);
> > + else
> > + strbuf_addstr(&state->stack->output, v->s);
> > return 0;
> > }
>
> may work as expected, but it makes readers wonder if there is
> significance to negative values other than -1 (in other words, what
> does it mean if v->s_size == -2, for example?).
>
Yes, any value less than -1 is meaningless here.
> It may make sense to
>
> * Turn atom_value.s_size field into ssize_t instead of size_t
>
Will the conversion of size_t and ssize_t break -Wsign-conversion?
Although there is a lot of code in Git that has broken it, but I am not
sure if it is wise to use ssize_t here.
> * Rewrite (v->s_size != -1) comparison to (v->s_size < 0)
>
For size_t, this scheme is wrong.
>
> Optionally we could introduce #define ATOM_SIZE_UNSPECIFIED (-1) and
> use it to initialize .s_size in ATOM_VALUE_INIT, but if we are just
> going to test "is it negative, then it is not given", then it probably
> is not needed.
>
It seems that this is the only method left. Although I think
ATOM_SIZE_UNSPECIFIED
is not very useful becasue we already have ATOM_VALUE_INIT.
> Other changes in the whole series relative to what has been queued
> looked all sensible to me.
>
> THanks.
>
Thanks.
--
ZheNing Hu
