All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dan Carpenter <error27@gmail.com>
To: Julia Lawall <julia.lawall@inria.fr>
Cc: Alex Elder <elder@ieee.org>,
	pavankumaryalagada@gmail.com, johan@kernel.org,
	gregkh@linuxfoundation.org, vireshk@kernel.org, elder@kernel.org,
	greybus-dev@lists.linaro.org, linux-staging@lists.linux.dev,
	linux-kernel@vger.kernel.org, outreachy@lists.linux.dev
Subject: Re: [PATCH v4] staging: greybus: replace PTR_ERR() with %pe in error messages
Date: Thu, 15 Oct 2026 22:57:01 +0300	[thread overview]
Message-ID: <atEwDf4RuTWtaSoW@stanley.mountain> (raw)
In-Reply-To: <cc868527-c13c-4e9b-aa7a-d2553f93d92@inria.fr>

On Thu, Oct 15, 2026 at 02:10:53PM -0400, Julia Lawall wrote:
> 
> 
> On Thu, 15 Oct 2026, Alex Elder wrote:
> 
> > On 10/15/26 3:22 AM, pavankumaryalagada@gmail.com wrote:
> > > From: Yalagada Pavan Kumar <pavankumaryalagada@gmail.com>
> > >
> > > Use %pe to print error pointers directly and display their
> > > error names.
> > >
> > > Keep PTR_ERR() calls when the numeric error value is needed
> > > for returning the error code.
> > >
> > > Signed-off-by: Yalagada Pavan Kumar <pavankumaryalagada@gmail.com>
> >
> > In a few places you are *adding* the error value where it was
> > missing (which I think is an improvement).  I assume that the
> > return values in this case are negative errnos.
> >
> > It would have been worth mentioning this other type of change
> > in the patch description too.  But it's not worth doing a new
> > version for that.  I trust you found all the spots, and fixed
> > them this way.  This looks good to me.
> 
> This Coccinelle semantic patch finds over 3000 opportunities for this
> change in the kernel as a whole.  Outside the context of outreachy, I
> wonder if people would be happy to see such changes, since the %pe code
> seems much more informative.
> 

I like %pe but it's not universally popular.  I've seen people NAK
these patches because it looks weird in their subsystem and I'm
wondering how many error messages their code prints?  Surely, any
error message would stand out is weird and undesirable?

regards,
dan carpenter


      reply	other threads:[~2026-10-15 19:57 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-15  8:22 [PATCH v4] staging: greybus: replace PTR_ERR() with %pe in error messages pavankumaryalagada
2026-10-15 12:13 ` Alex Elder
2026-10-15 18:10   ` Julia Lawall
2026-10-15 19:57     ` Dan Carpenter [this message]

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=atEwDf4RuTWtaSoW@stanley.mountain \
    --to=error27@gmail.com \
    --cc=elder@ieee.org \
    --cc=elder@kernel.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=greybus-dev@lists.linaro.org \
    --cc=johan@kernel.org \
    --cc=julia.lawall@inria.fr \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-staging@lists.linux.dev \
    --cc=outreachy@lists.linux.dev \
    --cc=pavankumaryalagada@gmail.com \
    --cc=vireshk@kernel.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.