Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1255386 > unrolled thread

RE: [char-misc 1/2 4.3 V2] mei: Fix debugfs filename in error output

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  RE: [char-misc 1/2 4.3 V2] mei: Fix debugfs filename in error output "Winkler, Tomas" <tomas.winkler@intel.com> - 2015-10-25 13:50 +0100
    Re: [char-misc 1/2 4.3 V2] mei: Fix debugfs filename in error output 'Greg Kroah-Hartman' <gregkh@linuxfoundation.org> - 2015-10-25 18:00 +0100

#1255386 — RE: [char-misc 1/2 4.3 V2] mei: Fix debugfs filename in error output

> 
> > > > >  		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?

Thanks
Tomas 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1255506

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@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web