linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Ivan Babrou <ivan@cloudflare.com>
To: Brian Foster <bfoster@redhat.com>
Cc: linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
	kernel-team@cloudflare.com, Alexey Dobriyan <adobriyan@gmail.com>,
	Al Viro <viro@zeniv.linux.org.uk>,
	"Theodore Ts'o" <tytso@mit.edu>,
	David Laight <David.Laight@aculab.com>,
	Jonathan Corbet <corbet@lwn.net>,
	Andrew Morton <akpm@linux-foundation.org>,
	David Hildenbrand <david@redhat.com>,
	Johannes Weiner <hannes@cmpxchg.org>,
	Christoph Anton Mitterer <mail@christoph.anton.mitterer.name>,
	Mike Rapoport <rppt@kernel.org>,
	Paul Gortmaker <paul.gortmaker@windriver.com>,
	Kalesh Singh <kaleshsingh@google.com>
Subject: Re: [PATCH v3] proc: report open files as size in stat() for /proc/pid/fd
Date: Tue, 18 Oct 2022 11:51:02 -0700	[thread overview]
Message-ID: <CABWYdi37Ts7KDshSvwMf34EKuUrz25duL7W8hOO8t1Xm53t2rA@mail.gmail.com> (raw)
In-Reply-To: <Y07taqdJ/J3EyJoB@bfoster>

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;
  }

  reply	other threads:[~2022-10-18 18:51 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-10-18  4:58 [PATCH v3] proc: report open files as size in stat() for /proc/pid/fd Ivan Babrou
2022-10-18 18:16 ` Brian Foster
2022-10-18 18:51   ` Ivan Babrou [this message]
2022-10-19 11:28     ` Brian Foster
2022-10-21  0:52       ` Andrew Morton
2022-10-18 23:13 ` Andrew Morton

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CABWYdi37Ts7KDshSvwMf34EKuUrz25duL7W8hOO8t1Xm53t2rA@mail.gmail.com \
    --to=ivan@cloudflare.com \
    --cc=David.Laight@aculab.com \
    --cc=adobriyan@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=bfoster@redhat.com \
    --cc=corbet@lwn.net \
    --cc=david@redhat.com \
    --cc=hannes@cmpxchg.org \
    --cc=kaleshsingh@google.com \
    --cc=kernel-team@cloudflare.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mail@christoph.anton.mitterer.name \
    --cc=paul.gortmaker@windriver.com \
    --cc=rppt@kernel.org \
    --cc=tytso@mit.edu \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).