linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Greg KH <gregkh@linuxfoundation.org>
To: Abdun Nihaal <abdun.nihaal@gmail.com>
Cc: Larry.Finger@lwfinger.net, phil@philpotter.co.uk,
	straube.linux@gmail.com, martin@kaiser.cx,
	linux-staging@lists.linux.dev, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3 1/2] staging: r8188eu: remove unneeded ret variables
Date: Thu, 27 Jan 2022 16:51:04 +0100	[thread overview]
Message-ID: <YfK/aOgEqftXuj1S@kroah.com> (raw)
In-Reply-To: <20220127152543.ttvp7gj5bzpuvr3p@nlap2>

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

  reply	other threads:[~2022-01-27 15:51 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-07 10:35 [PATCH v3 0/2] staging: r8188eu: remove unneeded ret variables Abdun Nihaal
2022-01-07 10:35 ` [PATCH v3 1/2] " Abdun Nihaal
2022-01-25 15:10   ` Greg KH
2022-01-27 15:25     ` Abdun Nihaal
2022-01-27 15:51       ` Greg KH [this message]
2022-01-29 10:20         ` Abdun Nihaal
2022-01-07 10:35 ` [PATCH v3 2/2] staging: r8188eu: change functions to return void Abdun Nihaal

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=YfK/aOgEqftXuj1S@kroah.com \
    --to=gregkh@linuxfoundation.org \
    --cc=Larry.Finger@lwfinger.net \
    --cc=abdun.nihaal@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-staging@lists.linux.dev \
    --cc=martin@kaiser.cx \
    --cc=phil@philpotter.co.uk \
    --cc=straube.linux@gmail.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;
as well as URLs for NNTP newsgroup(s).