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?):
> 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".
