linux-kernel - Re: [PATCH] staging:vme_user:fix the issue of using the wrong error code

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 11 Dec 2023 17:01:09 +0100
From:   Greg KH <gregkh@...uxfoundation.org>
To:     Piro Yang <piroyangg@...il.com>
Cc:     linux-staging@...ts.linux.dev,
        Linux Outreachy <outreachy@...ts.linux.dev>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] staging:vme_user:fix the issue of using the wrong error
 code

On Mon, Dec 11, 2023 at 11:54:53PM +0800, Piro Yang wrote:
> 1. the error code of ENOSYS indicates Invalid system call number,
>    but there is not system call
> 
> 2. unified the logging error message, when slave_set func is NULL

That is two different things, and as such, should be two different
changes, right?

Yes, it's picky, but that's what the staging driver code is for, to get
comfortable doing kernel development changes properly.

Also, are you sure this second change is correct:

> 
> Signed-off-by: Piro Yang <piroyangg@...il.com>
> ---
>  drivers/staging/vme_user/vme.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/vme_user/vme.c b/drivers/staging/vme_user/vme.c
> index 5c416c31ec57..e9461a7a7ab8 100644
> --- a/drivers/staging/vme_user/vme.c
> +++ b/drivers/staging/vme_user/vme.c
> @@ -340,8 +340,8 @@ int vme_slave_set(struct vme_resource *resource, int enabled,
>  	image = list_entry(resource->entry, struct vme_slave_resource, list);
>  
>  	if (!bridge->slave_set) {
> -		dev_err(bridge->parent, "Function not supported\n");
> -		return -ENOSYS;
> +		dev_err(bridge->parent, "%s not supported\n", __func__);

__func__ is not the same here as "Function", right?  "Function" is the
functionality of the thing that is attempted here, so replacing that
word with the function name seems odd to me, don't you think?

thanks,

greg k-h