mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Dan Carpenter <dan.carpenter@linaro.org>
To: Rae Moar <rmoar@google.com>
Cc: Richard Fitzgerald <rf@opensource.cirrus.com>,
	brendan.higgins@linux.dev, davidgow@google.com,
	linux-kselftest@vger.kernel.org, kunit-dev@googlegroups.com,
	linux-kernel@vger.kernel.org, patches@opensource.cirrus.com
Subject: Re: [PATCH v2 RESEND] kunit: debugfs: Handle errors from alloc_string_stream()
Date: Fri, 1 Dec 2023 10:50:50 +0300	[thread overview]
Message-ID: <a863d7dd-d982-462c-af5d-0ef0b48ea953@suswa.mountain> (raw)
In-Reply-To: <CA+GJov5vcRPo30CFYDwRq-iXHqtDtLd2jrmB+y_=BY6o2mZaBA@mail.gmail.com>

On Thu, Nov 30, 2023 at 04:11:18PM -0500, Rae Moar wrote:
> > +       stream = alloc_string_stream(GFP_KERNEL);
> > +       if (IS_ERR_OR_NULL(stream))
> 
> In response to Dan Carpenter's comment from the last version, I see
> the benefits of changing IS_ERR_OR_NULL() to IS_ERR() instead because
> "stream" will not be NULL. This would then also be the same as the
> check in kunit_alloc_string_stream.
> 
> However, I also see the benefit of checking for NULL just in case anyways.
> 

Returning NULL in alloc_string_stream() is a bug.  Checking for NULL is
a work around for bugs.  There are basically two times where it can
be valid to work around bugs like this instead of fixing them.  1)  When
the function is implemented by over 10 different driver authors.  In
that case you can guarantee that at least one of them is going to do the
wrong thing.  There are between 2-5 places which do this in the kernel.
2)  If it's a API that used to return NULL and it's changed to returning
error pointers.  I've never seen anyone do this, but I've proposed it as
a solution to make backporting easier.

regards,
dan carpenter


  reply	other threads:[~2023-12-01  7:51 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-10-30 10:47 Richard Fitzgerald
2023-11-30 21:11 ` Rae Moar
2023-12-01  7:50   ` Dan Carpenter [this message]
2023-12-01  3:42 ` David Gow

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=a863d7dd-d982-462c-af5d-0ef0b48ea953@suswa.mountain \
    --to=dan.carpenter@linaro.org \
    --cc=brendan.higgins@linux.dev \
    --cc=davidgow@google.com \
    --cc=kunit-dev@googlegroups.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=patches@opensource.cirrus.com \
    --cc=rf@opensource.cirrus.com \
    --cc=rmoar@google.com \
    /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

all inboxes | Powered by JetHome®