[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20151025165610.GB1374@kroah.com>
Date: Mon, 26 Oct 2015 01:56:10 +0900
From: 'Greg Kroah-Hartman' <gregkh@...uxfoundation.org>
To: "Winkler, Tomas" <tomas.winkler@...el.com>
Cc: "Usyskin, Alexander" <alexander.usyskin@...el.com>,
"'linux-kernel@...r.kernel.org'" <linux-kernel@...r.kernel.org>,
'Alexander Kuleshov' <kuleshovmail@...il.com>
Subject: Re: [char-misc 1/2 4.3 V2] mei: Fix debugfs filename in error output
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists