Messages in this thread | | | Date | Wed, 19 Oct 2022 07:28:45 -0400 | From | Brian Foster <> | Subject | Re: [PATCH v3] proc: report open files as size in stat() for /proc/pid/fd |
| |
On Tue, Oct 18, 2022 at 11:51:02AM -0700, Ivan Babrou wrote: > On Tue, Oct 18, 2022 at 11:16 AM Brian Foster <bfoster@redhat.com> wrote: > > > +static int proc_readfd_count(struct inode *inode) > > > +{ > > > + struct task_struct *p = get_proc_task(inode); > > > + struct fdtable *fdt; > > > + unsigned int open_fds = 0; > > > + > > > + if (!p) > > > + return -ENOENT; > > > > Maybe this shouldn't happen, but do you mean to assign the error code to > > stat->size in the caller? Otherwise this seems reasonable to me. > > You are right. As unlikely as it is to happen, we shouldn't return > negative size. > > What's the idiomatic way to make this work? My two options are: > > 1. Pass &stat->size into proc_readfd_count: > > if (S_ISDIR(inode->i_mode)) { > rv = proc_readfd_count(inode, &stat->size); > if (rv < 0) > goto out; > } > > out: > return rv; > > OR without a goto: > > if (S_ISDIR(inode->i_mode)) { > rv = proc_readfd_count(inode, &stat->size)); > if (rv < 0) > return rv; > } > > return rv; > > 2. Return negative count as error (as we don't expect negative amount > of files open): > > if (S_ISDIR(inode->i_mode)) { > size = proc_readfd_count(inode); > if (size < 0) > return size; > stat->size = size; > } >
I suppose the latter is less of a change to the original patch..? Either way seems reasonable to me. I have no strong preference FWIW.
Brian
| |