Re: [PATCH v2 02/16] odb: move list of packfiles into `struct packfile_store`

[Taylor Blau <me@xxxxxxxxxxxx>]

On Tue, Sep 02, 2025 at 10:50:41AM +0200, Patrick Steinhardt wrote:
> > > +void packfile_store_close(struct packfile_store *store)
> > > +{
> > > +	struct packed_git *p;
> > > +
> > > +	for (p = store->packs; p; p = p->next)
> > > +		if (p->do_not_close)
> > > +			BUG("want to close pack marked 'do-not-close'");
> > > +		else
> > > +			close_pack(p);
> > > +}
> >
> > And likewise this looks good to me. I do find the braceless for-loop a
> > little hard to read, but it's (a) correct, and (b) consistent with the
> > original implementation, so I don't feel strongly about changing it.
>
> Agreed, it is a bit awkward. I feel like our coding style should be
> amended to say that we only do braceless bodies in case the body is a
> single statement.

I think that our CodingGuidelines cover this as of 1797dc5176
(CodingGuidelines: clarify multi-line brace style, 2017-01-17), which
frowns upon statements that extend multiple lines.

So I think in this case, the CodingGuidelines would suggest that we
write this as:

    for (p = store->packs; p; p = p->next) {
        if (p->do_not_close)
            BUG("want to close pack marked 'do-not-close'");
        else
            close_pack(p);
    }

, which from our discussion here seems like something that we both find
more readable than the original.

Thanks,
Taylor