linux-kernel - Re: [PATCH v3 1/2] staging: r8188eu: remove unneeded ret variables

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:   Thu, 27 Jan 2022 16:51:04 +0100
From:   Greg KH <gregkh@...uxfoundation.org>
To:     Abdun Nihaal <abdun.nihaal@...il.com>
Cc:     Larry.Finger@...inger.net, phil@...lpotter.co.uk,
        straube.linux@...il.com, martin@...ser.cx,
        linux-staging@...ts.linux.dev, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 1/2] staging: r8188eu: remove unneeded ret variables

On Thu, Jan 27, 2022 at 08:55:43PM +0530, Abdun Nihaal wrote:
> Hello Greg,
> 
> On Tue, Jan 25, 2022 at 04:10:57PM +0100, Greg KH wrote:
> > Many, if not most, of these functions should either be having their
> > return value checked, or be void functions as no one checks their return
> > value and they can not fail.  Please split this up and look at each
> > function to determine which is is and how to fix it up properly.  Just
> > returning 0 all the time is not the correct thing to do all the time.
> > 
> > One example would be rtw_p2p_get_status()  It can not fail, so why does
> > it return anything?
> 
> Thanks for reviewing the patches.
> 
> I had split the changes in a way that the first patch removes the
> unneeded return variables and the second patch converts the functions
> (changed by the first patch) whose return values are not used, to return
> void.
> 
> But yes, I now think, it is better to just convert the functions
> whose return values are not used, directly to return void instead of
> first removing the unneeded return variable and then converting
> to return void.
> 
> I'll resend this as a single patch.

A single patch per function you are changing is a good idea.  Do not mix
them all together, would you want to review something like that?

Always remember that you want to make changes obvious and simple to
review.

thanks,

greg k-h