Re: [PATCH 10/13] eeepc-laptop: compare proper return values in get_cpufv

[Darren Hart]

On Tue, Sep 16, 2014 at 01:52:47PM -0700, Darren Hart wrote:
> On Tue, Sep 16, 2014 at 01:54:25PM +0200, Frans Klaver wrote:
> > On Mon, Sep 15, 2014 at 11:55 PM, Frans Klaver <fransklaver@xxxxxxxxx> wrote:
> > > On Mon, Sep 15, 2014 at 02:51:25PM -0700, Greg Kroah-Hartman wrote:
> > >> On Mon, Sep 15, 2014 at 02:49:02PM -0700, Darren Hart wrote:
> > >> >
> > >> > This patch is fine as is. However, Greg has supported propogating the error code
> > >> > through to the sysfs interface (if I understand him correctly on an earlier post
> > >> > to this list). This would require an addition change to this patch would
> > >> > propogated the get_cpufv error code in show_available_cpuv(), show_cpuv(), and
> > >> > store_cpuv(). As it is, we return -ENODEV on any failure, where an ACPI call
> > >> > error should probably return -ENXIO as I understand it.
> > >>
> > >> I really have no idea at this point in time what to recommend.  How
> > >> about just stick with what is happening today so that:
> > >>
> > >> > However, there was a rather famous change in error code handling in which pulse
> > >> > audio broke and Linus was very upset with one of his maintainers.
> > >>
> > >> That doesn't happen :)
> > >
> > > So if I interpret that correctly, we're dropping the last patch
> > > (ENODEV -> ENXIO) from the series? That's fine by me. As mentioned
> > > earlier, I already saw something else break because I returned ENXIO
> > > instead of ENODEV.
> > >
> > > Maybe it's a good idea to try and document the expected behavior
> > > somewhere, if even Greg isn't sure what to do.
> > 
> > For good measure:
> > 
> > v2 will not change the return values at the sysfs interface, meaning
> > we will always return -ENODEV on error. I am going to try to keep as
> > much internal functions propagating errors as possible though, unless
> > someone strongly disagrees.
> > 
> > Thanks,
> > Frans
> 
> I cornered Linus today and asked about this specifically. The policy is this:
> 
> Don't change the sysfs return codes without good reason. A good reason could be
> a real bug or problem with the return codes. It could also be to consolidate
> error handling which makes things more uniform, etc.
> 
> If this results in broken userspace, the maintainer will revert the change.
> 
> This is probably a good thing to add to sysfs-rules.txt. I'll prepare a patch.
> 

What do people think of appending this to sysfs-rules.txt?

- When reading and writing sysfs device attribute files, avoid dependency
  on specific error codes wherever possible. This minimizes coupling to
  the error handling implemementation within the kernel.

  In general, failures to read or write sysfs device attributes shall
  propogate errors wherever possible. Common errors include, but are not
  limited to:

  -EIO: The read or store operation is not supported, typically returned by
        the sysfs system itself if the read or store pointer is NULL.

  -ENXIO: The read or store operation failed

  Error codes will not be changed without good reason, and should a change
  to error codes result in user-space breakage, it will be fixed, or the
  the offending change will be reverted.

  Userspace applications can, however, expect the format and contents of
  the attribute files to remain consistent in the absence of a version
  attribute change in the context of a given attributes.

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