Message ID | 20200508153634.249933-10-hch@lst.de (mailing list archive) |
---|---|
State | Changes Requested |
Headers | show |
Series | [01/12] fd: add a new __anon_inode_getfd helper | expand |
On Fri, May 08, 2020 at 05:36:31PM +0200, Christoph Hellwig wrote: > Use __anon_inode_getfd instead of opencoding the logic using > get_unused_fd_flags + anon_inode_getfile. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- > drivers/infiniband/core/rdma_core.c | 17 ++++------------- > 1 file changed, 4 insertions(+), 13 deletions(-) > diff --git a/drivers/infiniband/core/rdma_core.c b/drivers/infiniband/core/rdma_core.c > index 5128cb16bb485..541e5e06347f6 100644 > --- a/drivers/infiniband/core/rdma_core.c > +++ b/drivers/infiniband/core/rdma_core.c > @@ -462,30 +462,21 @@ alloc_begin_fd_uobject(const struct uverbs_api_object *obj, > if (WARN_ON(fd_type->fops->release != &uverbs_uobject_fd_release)) > return ERR_PTR(-EINVAL); > > - new_fd = get_unused_fd_flags(O_CLOEXEC); > - if (new_fd < 0) > - return ERR_PTR(new_fd); > - > uobj = alloc_uobj(attrs, obj); > if (IS_ERR(uobj)) > - goto err_fd; > + return uobj; > > /* Note that uverbs_uobject_fd_release() is called during abort */ > - filp = anon_inode_getfile(fd_type->name, fd_type->fops, NULL, > - fd_type->flags); > - if (IS_ERR(filp)) { > - uobj = ERR_CAST(filp); > + new_fd = __anon_inode_getfd(fd_type->name, fd_type->fops, NULL, > + fd_type->flags | O_CLOEXEC, &filp); > + if (new_fd < 0) > goto err_uobj; This will conflict with a fix (83a267021221 'RDMA/core: Fix overwriting of uobj in case of error') that is going to go to -rc soon. Also the above misses returning an ERR_PTR if __anon_inode_getfd fails, it returns a uobj that had been freed.. I suppose it should be something like if (new_fd < 0) { uverbs_uobject_put(uobj); return ERR_PTR(new_fd) } ? Jason
diff --git a/drivers/infiniband/core/rdma_core.c b/drivers/infiniband/core/rdma_core.c index 5128cb16bb485..541e5e06347f6 100644 --- a/drivers/infiniband/core/rdma_core.c +++ b/drivers/infiniband/core/rdma_core.c @@ -462,30 +462,21 @@ alloc_begin_fd_uobject(const struct uverbs_api_object *obj, if (WARN_ON(fd_type->fops->release != &uverbs_uobject_fd_release)) return ERR_PTR(-EINVAL); - new_fd = get_unused_fd_flags(O_CLOEXEC); - if (new_fd < 0) - return ERR_PTR(new_fd); - uobj = alloc_uobj(attrs, obj); if (IS_ERR(uobj)) - goto err_fd; + return uobj; /* Note that uverbs_uobject_fd_release() is called during abort */ - filp = anon_inode_getfile(fd_type->name, fd_type->fops, NULL, - fd_type->flags); - if (IS_ERR(filp)) { - uobj = ERR_CAST(filp); + new_fd = __anon_inode_getfd(fd_type->name, fd_type->fops, NULL, + fd_type->flags | O_CLOEXEC, &filp); + if (new_fd < 0) goto err_uobj; - } uobj->object = filp; - uobj->id = new_fd; return uobj; err_uobj: uverbs_uobject_put(uobj); -err_fd: - put_unused_fd(new_fd); return uobj; }
Use __anon_inode_getfd instead of opencoding the logic using get_unused_fd_flags + anon_inode_getfile. Signed-off-by: Christoph Hellwig <hch@lst.de> --- drivers/infiniband/core/rdma_core.c | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-)