Junio C Hamano <gitster@xxxxxxxxx> 于2021年6月16日周三 下午3:49写道：
>
> "ZheNing Hu via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:
>
> > From: ZheNing Hu <adlternative@xxxxxxxxx>
> >
> > Introduce free_array_item_internal() for freeing ref_array_item value.
> > It will be called internally by free_array_item(), and it will help
> > `cat-file --batch` free ref_array_item's memory later.
>
> As a file local static function, the horrible name free_array_item()
> was tolerable. But before exposing a name like that to the outside
> world, think twice if that is specific enough, and it is not. There
> are 47 different kinds of "array"s we use in the system, but this
> new helper function only works with ref_array_item and not on items
> in any other kinds of arrays.
>
Well, this is indeed ignored by me.
So free_array_item() --> free_ref_array_item() and free_array_item()
--> free_ref_array_item().
> > -/* Free memory allocated for a ref_array_item */
> > -static void free_array_item(struct ref_array_item *item)
> > +void free_array_item_internal(struct ref_array_item *item)
> > {
>
> And "internal" is a horrible name to have as an external name. You
> probably can come up with a more appropriate name when you imagine
> yourself explaining to somebody who is relatively new to this part
> of the codebase what the difference between free_array_item() and
> this new helper is, where the difference comes from, why the symref
> member (and no other member) is so special, etc.
>
Yeah, "internal" is an incorrect naming. The main purpose here is
to only free a ref_array_item's value.
> I _think_ what is special is not the .symref but is the .value
> field, IOW, you are trying to come up with an interface to free the
> value part of ref_array_item without touching other things. But it
> is not helpful at all to readers if you do not explain why you want
> to do so. Why is the .value member so special? The ability to
> clear only the .value member without touching other members is useful
> because ...?
>
Because batch_object_write() use a ref_array_item which is allocated on the
stack, and it's member symref is not used at all. ref_array_item's symref and
itself do not need free. The original free_array_item() will free all
dynamically
allocated content in ref_array_item. It cannot meet our requirements
at this time:
only free the ref_array_item's value.
> In any case, assuming that you'd establish why the .value member is
> so special to deserve an externally callable function, when external
> callers do not have to be able to free the item as a whole (i.e.
> free_array_item() is still file-scope static), in the proposed log
> message in an updated patch, I would imagine that
>
> free_ref_array_item_value()
>
> would be a more suitable name than the _internal thing. When it
> happens, you might want to rename the static one to
> free_ref_array_item() to match, even if it does not have external
> callers.
Make sence.
--
ZheNing Hu
