* [char_dev] Invalid uses of cdev_add return value
@ 2023-07-07 16:55 Roi L
2023-07-07 17:31 ` Matthew Wilcox
0 siblings, 1 reply; 2+ messages in thread
From: Roi L @ 2023-07-07 16:55 UTC (permalink / raw
To: linux-fsdevel
I hope I'm emailing the right place. I have recently noticed that there
are many invalid uses of `cdev_add`'s return value in the kernel source.
While `cdev_add` clearly mentions that it returns a negative value on
failure, many calls to this function check the return value as a
positive value.
E.g. (/drivers/mtd/ubi/vmt.c:581)
```
err = cdev_add(&vol->cdev, dev, 1);
if (err) {
ubi_err(ubi, "cannot add character device for volume %d, error %d",
vol_id, err);
...
```
Also, there are a few sources who check for a negative value, `err < 0`. I
I suspect these are indeed invalid usages of the function, though I'm not sure that's why I'm contacting you.
--
Regards, Roi L (roeilev321_@outlook.com)
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [char_dev] Invalid uses of cdev_add return value
2023-07-07 16:55 [char_dev] Invalid uses of cdev_add return value Roi L
@ 2023-07-07 17:31 ` Matthew Wilcox
0 siblings, 0 replies; 2+ messages in thread
From: Matthew Wilcox @ 2023-07-07 17:31 UTC (permalink / raw
To: Roi L; +Cc: linux-fsdevel
On Fri, Jul 07, 2023 at 07:55:28PM +0300, Roi L wrote:
> I hope I'm emailing the right place. I have recently noticed that there
> are many invalid uses of `cdev_add`'s return value in the kernel source.
> While `cdev_add` clearly mentions that it returns a negative value on
> failure, many calls to this function check the return value as a
> positive value.
>
> E.g. (/drivers/mtd/ubi/vmt.c:581)
> ```
> err = cdev_add(&vol->cdev, dev, 1);
> if (err) {
> ubi_err(ubi, "cannot add character device for volume %d, error %d",
> vol_id, err);
OK, what you're missing is that on success, cdev_add() returns zero.
So in practice, there is no difference between 'if (err)' and
'if (err < 0)'. Both check for an error, as err can never be positive.
I happen to think it's good practice to check for err < 0 rather than
just err, and it makes no difference at all on any CPU that I've ever
encountered. But patches to clean it up would be unwelcome churn.
Thanks for checking rather than starting out by sending patches that
would have to be rejected!
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2023-07-07 17:31 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-07-07 16:55 [char_dev] Invalid uses of cdev_add return value Roi L
2023-07-07 17:31 ` Matthew Wilcox
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.