On Mon, Dec 07, 2020 at 10:33:00PM +0800, Gao Xiang wrote:
> Hi Christoph,
>
> On Mon, Dec 07, 2020 at 02:56:42PM +0100, Christoph Hellwig wrote:
> > > if (pag->pagi_freecount) {
> > > xfs_perag_put(pag);
> > > + *IO_agbp = agbp;
> > > + return 0;
> >
> > I think assigning *IO_agbp would benefit from a little consolidation.
> > Set it to NULL in the normal unsuccessful return, and add a found_ag
> > label that assigns agbp and returns 0.
>
> Just to confirm the main idea, I think it might be:
>
> *IO_agbp = NULL; at first,
>
> and combine all such assignment
> > > + *IO_agbp = agbp;
> > > + return 0;
> >
>
> into a new found_ag lebel, and use goto found_ag; for such cases.
> Do I understand correctly? If that is correct, will update
> in the next version.
I would also move
*IO_agbp = NULL;
to just before the
return error;
to match the assignment for the successful case, but that isn't
the important part. I think the important part is to have on
central place for the sucessful return.
