From: quic_zijuhu <quic_zijuhu@quicinc.com>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Zijun Hu <zijun_hu@icloud.com>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>, <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] driver core: bus: Return -EIO instead of 0 when show/store invalid bus attribute
Date: Wed, 24 Jul 2024 20:56:18 +0800 [thread overview]
Message-ID: <70ec36ff-1be9-4fc3-9792-4950a483b272@quicinc.com> (raw)
In-Reply-To: <2024072316-yeah-snide-ca28@gregkh>
On 7/24/2024 1:31 AM, Greg Kroah-Hartman wrote:
> On Tue, Jul 23, 2024 at 10:55:43PM +0800, Zijun Hu wrote:
>> From: Zijun Hu <quic_zijuhu@quicinc.com>
>>
>> Return -EIO instead of 0 when show/store invalid bus attribute as
>> class/device/driver/kobject attribute.
>
> Why? What is this now going to break? You are changing a user-visable
> api that has been this way for 20+ years, how was this tested?
>
this change should break nothing.
tested by wc a writing only bus attribute, for example
root@kvm-Q35:/sys/bus/gpio# ls -l
--w------- 1 root root 4096 7月 24 20:20 drivers_probe
root@kvm-Q35:/sys/bus/gpio# chmod u+r drivers_probe
root@kvm-Q35:/sys/bus/gpio# wc -c drivers_probe
0 drivers_probe // for current design
root@zijun-kvm-Q35:/sys/bus/gpio# wc -c drivers_probe
wc: drivers_probe: Input/output error // for this change
>>
>> Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>
>> ---
>> drivers/base/bus.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/base/bus.c b/drivers/base/bus.c
>> index ffea0728b8b2..e84522a90102 100644
>> --- a/drivers/base/bus.c
>> +++ b/drivers/base/bus.c
>> @@ -152,7 +152,7 @@ static ssize_t bus_attr_show(struct kobject *kobj, struct attribute *attr,
>> {
>> struct bus_attribute *bus_attr = to_bus_attr(attr);
>> struct subsys_private *subsys_priv = to_subsys_private(kobj);
>> - ssize_t ret = 0;
>> + ssize_t ret = -EIO;
>>
>> if (bus_attr->show)
>> ret = bus_attr->show(subsys_priv->bus, buf);
>> @@ -164,7 +164,7 @@ static ssize_t bus_attr_store(struct kobject *kobj, struct attribute *attr,
>> {
>> struct bus_attribute *bus_attr = to_bus_attr(attr);
>> struct subsys_private *subsys_priv = to_subsys_private(kobj);
>> - ssize_t ret = 0;
>> + ssize_t ret = -EIO;
>
> Why -EIO? What is wrong with returning an empty string?
>
for this error case, all class/device/driver/kobject attributes return
-EIO, i just follow them for bus attribute.
for empty string, the value returned by attribute store() is returned.
> I think I know why you picked this but you better document the heck out
> of it AND test it very very well first.
>
sure.
> thanks,
>
> greg k-h
next prev parent reply other threads:[~2024-07-24 12:56 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-07-23 14:55 Zijun Hu
2024-07-23 17:31 ` Greg Kroah-Hartman
2024-07-24 12:56 ` quic_zijuhu [this message]
2024-07-24 13:29 ` Greg Kroah-Hartman
2024-07-24 14:17 ` quic_zijuhu
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=70ec36ff-1be9-4fc3-9792-4950a483b272@quicinc.com \
--to=quic_zijuhu@quicinc.com \
--cc=gregkh@linuxfoundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=rafael@kernel.org \
--cc=zijun_hu@icloud.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®