On Mon, 19 Feb 2018 09:50:24 +0000 Shameerali Kolothum Thodi <shameerali.kolothum.thodi@xxxxxxxxxx> wrote: > > -----Original Message----- > > From: Alex Williamson [mailto:alex.williamson@xxxxxxxxxx] > > Sent: Friday, February 16, 2018 8:49 PM > > On Thu, 15 Feb 2018 09:44:59 +0000 > > Shameer Kolothum <shameerali.kolothum.thodi@xxxxxxxxxx> wrote: > > > + node->end = end; > > > + continue; > > > + } > > > + /* Delete nodes after new end */ > > > + list_del(&node->list); > > > + kfree(node); > > > + } > > > + > > > + return 0; > > > +} > > > + > > > +static int vfio_iommu_get_iova_copy(struct vfio_iommu *iommu, > > > + struct list_head *iova_copy) > > > +{ > > > + > > > + struct list_head *iova = &iommu->iova_list; > > > + struct vfio_iova *n; > > > + > > > + list_for_each_entry(n, iova, list) { > > > + int ret; > > > + > > > + ret = vfio_insert_iova(n->start, n->end, iova_copy); > > > + if (ret) > > > + return ret; > > > > Let's delete and free any entries added to the copy here too. > > Ok. My original thought was caller will free up in case of error. This comes down to Rusty's suggestions of how to make an API hard to misuse rather than simply easy to use to me. Placing the onus on the caller to cleanup a list sounds simple, but the caller passed an empty list and the function failed, why should the caller bother to check if the function left any cruft on the list in the course of failing? This is not a hard to misuse interface, in fact it's very easy to forget that cleanup. Thanks, Alex
