On Mon, Nov 18, 2024 at 08:40:34AM +0100, Patrick Steinhardt wrote:
> > static struct cached_object *find_cached_object(const struct object_id *oid)
> > {
> > + static struct cached_object empty_tree = {
> > + /* no oid needed; we'll look it up manually based on the_hash_algo */
> > + .type = OBJ_TREE,
> > + .buf = "",
> > + };
> > int i;
> > struct cached_object *co = cached_objects;
>
> I was wondering whether we want to also mark this as `const` so that no
> caller ever gets the idea of modifying the struct. Something like the
> below patch (which applies on "master", so it of course would have to
> adapt to your changes).
This seems like a fairly unlikely bug to me, just because it would be
weird for somebody to want to write to the response (whereas the other
future-proofing was against somebody reading a private-ish value).
Still, I agree that "const" is the right thing, and it's not hard to add
it in to my series.
> diff --git a/object-file.c b/object-file.c
> index b1a3463852..f15a3f6a5f 100644
> --- a/object-file.c
> +++ b/object-file.c
> @@ -321,7 +321,7 @@ static struct cached_object {
> } *cached_objects;
> static int cached_object_nr, cached_object_alloc;
>
> -static struct cached_object empty_tree = {
> +static const struct cached_object empty_tree = {
> .oid = {
> .hash = EMPTY_TREE_SHA1_BIN_LITERAL,
> },
This hunk is technically not needed since we can implicitly cast from
non-const to const when returning. I included it, though, along with
making the iteration pointer in find_cached_object() const since that
better represents the intent.
-Peff
