On Mon, 12 Nov 2018 11:41:50 +0100 Pierre Morel <pmorel@xxxxxxxxxxxxx> wrote: > On 12/11/2018 11:32, Cornelia Huck wrote: > > 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? > > > > At the end,pfn_array_alloc_pin() is decoupled in pfn_array_alloc() and > pfn_array_pin() which is a good thing but but the pfn_array_unpin_free() > survives as unpin + free. OK, this seems like a good idea to me, then :)
