linux-kernel - Re: [PATCH] staging: gdm724x: fix returning -1 with return equivalent errors

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] [day] [month] [year] [list]

Message-ID: <47723ac6-9131-49f2-8746-1bc8845187cf@stanley.mountain>
Date: Fri, 4 Oct 2024 11:00:23 +0300
From: Dan Carpenter <dan.carpenter@...aro.org>
To: Rodrigo Gobbi <rodrigo.gobbi.7@...il.com>
Cc: jonathankim@...semi.com, deanahn@...semi.com,
	gregkh@...uxfoundation.org, ~lkcamp/patches@...ts.sr.ht,
	linux-staging@...ts.linux.dev, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] staging: gdm724x: fix returning -1 with return
 equivalent errors

On Thu, Oct 03, 2024 at 11:20:25PM -0300, Rodrigo Gobbi wrote:
> As in the TODO file, use proper error codes from PM callbacks and init.
> 
> Signed-off-by: Rodrigo Gobbi <rodrigo.gobbi.7@...il.com>
> ---
> Only one reference was left, regarding the packet_type_to_tty_index() but
> I think it's reasonable to keep it that way since it's for tty index purpose.

Better to return an error code there as well.  It doesn't change runtime
behavior at all, we're just having a discussoin about cleanliness.  I think
error codes are more clean.

> 
> There is a RFC to delete this driver at [1], so I'm not 
> sure if this change is worth it. Anyway, I'm submitting 
> it and I'll be waiting for an opinion about this.
> Tks.
> 
> [1] https://lore.kernel.org/lkml/50020db0-3bad-41f5-8da3-c66bc0a90fe6@gmail.com/

Most likely Greg will keep merging patches until the driver is removed.  It's
up to you to decide if it's worth your time to keeps sending the patches.

> @@ -991,7 +991,7 @@ static int __init gdm_usb_lte_init(void)
>  {
>  	if (gdm_lte_event_init() < 0) {
>  		pr_err("error creating event\n");
> -		return -1;
> +		return -ENODEV;
>  	}

This should be:

	ret = gdm_lte_event_init();
	if (ret < 0) {
		pr_err("error creating event\n");
		return ret;
	}

regards,
dan carpenter