On Wed, 19 Oct 2022 07:28:45 -0400 Brian Foster <bfoster@xxxxxxxxxx> wrote: > 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@xxxxxxxxxx> 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. If get_proc_task() failed then something has gone horridly wrong, hasn't it? Wouldn't it make sense in this situation to make the .getattr() itself return an errno, in which case the data at *stat dosen't matter - it's invalid anyway. This seems to be the general approach in procfs when get_proc_task() fails - return -ESRCH (or, seemingly randomly, -ENOENT) to the caller.