On Mon, 12 Nov 2018 11:28:38 +0100 Pierre Morel <pmorel@xxxxxxxxxxxxx> wrote: > Hi Eric, > > I think the problem here comes from the pfn_array_table_unpin_free() > doing both unpin and free. > > I would prefer that you change the pfn_array_table_init() to return the > pointer, so you can free the pointer in the caller like: > > > p = pfn_array_table_init(pat, 1); > if (!p) > goto out_init; > > ret = pfn_array_alloc_pin(pat->pat_pa, cp->mdev, ccw->cda, > ccw->count); > if (ret < 0) > goto out_free; > ... > > out_unpin: > pfn_array_table_unpin_free(pat, cp->mdev); > out_free: > pfn_array_table_free(p); > out_init: > ccw->cda = 0; > return ret; > } > > > And modify the pfn_array_table_unpin_free() to pfn_array_table_unpin() > and add the freeing in pfn_array_table_free(p). > > > Something like that with a correct return code handle. > > Which would make the code more logical and readable. > > What do you think? While I agree that this would improve the code, I'm not sure how much of it remains at the end of this series (I haven't read it completely yet.) IOW, is this a code change that would get kicked out again anyway?
