mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "'Greg Kroah-Hartman'" <gregkh@linuxfoundation.org>
To: "Winkler, Tomas" <tomas.winkler@intel.com>
Cc: "Usyskin, Alexander" <alexander.usyskin@intel.com>,
	"'linux-kernel@vger.kernel.org'" <linux-kernel@vger.kernel.org>,
	"'Alexander Kuleshov'" <kuleshovmail@gmail.com>
Subject: Re: [char-misc 1/2 4.3 V2] mei: Fix debugfs filename in error output
Date: Mon, 26 Oct 2015 01:56:10 +0900	[thread overview]
Message-ID: <20151025165610.GB1374@kroah.com> (raw)
In-Reply-To: <5B8DA87D05A7694D9FA63FD143655C1B41E31E59@hasmsx108.ger.corp.intel.com>

On Sun, Oct 25, 2015 at 12:45:30PM +0000, Winkler, Tomas wrote:
> > 
> > > > > >  		goto err;
> > > > >
> > > > > You should never care if a debugfs call fails or not.
> > > >
> > > > The system should not be dependent on the debug feature but, it is
> > > > always good to know if there our system is failing
> > >
> > > And what can you do if it is "failing"?  Really nothing, so there's
> > > nothing to check here.
> > 
> > As far as I can see the function debugfs_create_file may fail for a few reasons and
> > it return  NULL if this is happening.
> > I might ignore the error as you suggested, but I'm not sure why not to give a hint
> > in log that this happened.
> > Second, as far as I scanned the kernel sources, checking the return value of this
> > function and acting on this is very common.
> > 
> > > >   Also, this will
> > > > > "fail" if you don't have CONFIG_DEBUGFS enabled, which means you are
> > > > > using the api wrong :(
> > > >
> > > > The whole file is not compiled if CONFIG_DEBUGFS is not set, please see the
> > > Makefile
> > >
> > > Ok, then you don't need to check anything.  Debugfs was created to be
> > > dirt-simple, don't add complexity and "must unwind cleanly" type logic
> > > here where it's not needed at all.  That just hurts my sensibilities for
> > > why I made the API the way it is in the first place :)
> > 
> > I don't see the code much complex, it is pretty much boilerplate code and this is
> > why this c&p error had happened.
> > So this patch just fixes a c&p error in an error message and if we wish to change
> > the behavior to match your vision
> > I suggest to doing it another patch set... maybe even sweeping the whole kernel.
> 
> 
> Greg, are you merging this patch? What is your final say?

It's no longer in my todo queue, so there's nothing for me to apply
here, sorry.

Please resend it if you feel it should be added.

thanks,

greg k-h

      reply	other threads:[~2015-10-25 16:56 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-09-21  7:58 Tomas Winkler
2015-10-04 11:19 ` Greg Kroah-Hartman
2015-10-05 14:47   ` Winkler, Tomas
2015-10-05 15:30     ` Greg Kroah-Hartman
2015-10-06  8:46       ` Winkler, Tomas
2015-10-25 12:45         ` Winkler, Tomas
2015-10-25 16:56           ` 'Greg Kroah-Hartman' [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=20151025165610.GB1374@kroah.com \
    --to=gregkh@linuxfoundation.org \
    --cc=alexander.usyskin@intel.com \
    --cc=kuleshovmail@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tomas.winkler@intel.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®