Junio C Hamano <gitster@xxxxxxxxx> 于2021年6月3日周四 上午10:10写道：
>
> "ZheNing Hu via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:
>
> > /* See grab_values */
> > -static void grab_sub_body_contents(struct atom_value *val, int deref, void *buf)
> > +static void grab_sub_body_contents(struct atom_value *val, int deref, void *buf,
> > + struct object *obj)
>
> Neither this step or the next change needs anything but type member
> of the 'obj' (and 'buf' is coming from oi.content of the result of
> asking about that same 'obj').
>
> I wonder if we should do one of the following:
>
> (1) stop passing "void *buf" and instead "struct expand_data
> *data", and use "data->content" to access "buf", which would
> allow you to access "data->type" to perform the added check.
>
> (2) instead of adding "struct obj *obj" to the parameters, just add
> "enum object_type type", as that is the only thing you need.
>
> Obviously (2) is with lessor impact, but if it can be done safely
> without breaking the code [*], (1) would probably be a much more
> preferrable direction to go in the longer term.
>
I agree with (1). In future versions of grab_sub_body_contents(), we will
use the content of "data" more frequently instead of using the
crude "obj". The type provided by "obj" can also be provided by
"data". So yes, I would be very willing to let grab_sub_body_contents()
only use "data". (delete "obj")
E.g.
static void grab_sub_body_contents(struct atom_value *val, int deref,
struct expand_data *data)
Using (2), we will need more parameters to pass other object info.
> Side note [*]. A caller is allowed to choose to feed "buf" that
> is different from "oi.content" (perhaps buf may sometimes want
> to be a utf-8 recoded version of oi.content for certain types of
> objects) with the current system, but if we pass expand_data
> throughout the callchain, such a caller is broken, for example.
>
Just see the situation in front of us: grab_sub_body_contents()
have only one caller: grab_values(). If someone need a function like
grab_sub_body_contents() to grab another buf, they can use rewrite
a more universal function interface:
static void grab_sub_body_contents(struct atom_value *val, int deref,
struct expand_data *data)
{
grab_sub_body_contents_internal(val, deref, data->content,
data->size, data->type);
}
static void grab_sub_body_contents_internal(struct atom_value *val,
int deref, void *buf,
unsigned long buf_size,
enum object_type type)
{
...
}
But for the time being, the above one is sufficient.
Thanks.
--
ZheNing Hu
