"ZheNing Hu via GitGitGadget" [off-list ref] 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.
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.
Junio C Hamano [off-list ref] 于2021年6月3日周四 上午10:10写道：
"ZheNing Hu via GitGitGadget" [off-list ref] writes:
quoted
/* 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