Ævar Arnfjörð Bjarmason [off-list ref] 于2021年7月22日周四 下午4:10写道：

On Thu, Jul 22 2021, ZheNing Hu via GitGitGadget wrote:

quoted

[..] 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:

quoted

 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.

quoted

      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