public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Richard Purdie <rpurdie@rpsys.net>
To: Johannes Berg <johannes@sipsolutions.net>
Cc: Pavel Machek <pavel@ucw.cz>,
	Linux Kernel list <linux-kernel@vger.kernel.org>,
	John Lenz <lenz@cs.wisc.edu>,
	Richard Purdie <rpurdie@openedhand.com>
Subject: Re: led_class: storing a value can act but return -EINVAL
Date: Sun, 30 Apr 2006 12:57:49 +0100	[thread overview]
Message-ID: <1146398270.6254.9.camel@localhost.localdomain> (raw)
In-Reply-To: <1146394862.5019.53.camel@localhost>

On Sun, 2006-04-30 at 13:01 +0200, Johannes Berg wrote:
> Hi,
> 
> > Well, I'd argue current behaviour is okay... can you strace it? It
> > should accept the number (return 3) then return -EINVAL.
> 
> That's exactly what happens.
> 
> Which is totally bogus, because userspace will think that the setting
> didn't succeed. Or application authors will ignore the return value
> assuming that it always succeeded. Or read the value back to see if it
> succeeded. All icky, when we can well have a good return value.
> 
> > > There are two possible ways to handle this:
> > > a) accept anything that begins with a valid number.
> > > b) reject anything that isn't *only* a number
> > 
> > c) accept anything that is number, ignore newlines.

echo 255> brightness works, returns success.
echo 255 > brightness works but then returns -EINVAL.

So we currently do b, quite strictly. Its the trailing space thats the
problem. It also shouldn't have altered the brightness value if it ends
up returning -EINVAL.

I've looked around other implementations and it would appear we should
accept an optional space. Most sysfs attributes seem to handle this
differently, each with its own "bugs".

I've some fixes in mind both for the led and backlight classes which
I'll post once I've done a little more testing. I'd be interested to
know the official view on what the attributes should/shouldn't accept
is.

Cheers,

Richard


  reply	other threads:[~2006-04-30 11:58 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-04-29 11:33 led_class: storing a value can act but return -EINVAL Johannes Berg
2006-04-30 10:02 ` Pavel Machek
2006-04-30 11:01   ` Johannes Berg
2006-04-30 11:57     ` Richard Purdie [this message]
2006-04-30 12:05       ` Johannes Berg
2006-04-30 12:10     ` Pavel Machek

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=1146398270.6254.9.camel@localhost.localdomain \
    --to=rpurdie@rpsys.net \
    --cc=johannes@sipsolutions.net \
    --cc=lenz@cs.wisc.edu \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pavel@ucw.cz \
    --cc=rpurdie@openedhand.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