Re: [Outreachy kernel] [PATCH 1/4] staging: rtl8723bs: core: Remove true and false comparison

From: Dan Carpenter
Date: Wed Nov 03 2021 - 07:53:21 EST


On Fri, Oct 22, 2021 at 08:14:29AM +0530, Praveen Kumar wrote:
> On 22-10-2021 01:27, Kushal Kothari wrote:
> > diff --git a/drivers/staging/rtl8723bs/core/rtw_cmd.c b/drivers/staging/rtl8723bs/core/rtw_cmd.c
> > index 2624e994513f..0be3a8dbeec8 100644
> > --- a/drivers/staging/rtl8723bs/core/rtw_cmd.c
> > +++ b/drivers/staging/rtl8723bs/core/rtw_cmd.c
> > @@ -371,7 +371,7 @@ void rtw_free_cmd_obj(struct cmd_obj *pcmd)
> > void rtw_stop_cmd_thread(struct adapter *adapter)
> > {
> > if (adapter->cmdThread &&
> > - atomic_read(&(adapter->cmdpriv.cmdthd_running)) == true &&
> > + atomic_read(&adapter->cmdpriv.cmdthd_running) &&
>
> Lets keep it "atomic_read(&(adapter->cmdpriv.cmdthd_running))"
> and remove the parentheses in next patch.
>

There is a certain amount of flexibility in fixing style errors which
occur on a line that you are already modifying. But your advice is good
because 1) Then the inclination is to say "please fix the alignment as
well". 2) There is already a [PATCH 2/4] which removes these extra
parentheses so it makes more sense to do it there. 3) This patchset
will need to be redone anyway because of the white space error in
[PATCH 4/4].

So, you are right, and especially in this case you're right. But there
are always going to be minor issues with a patch and some of them we can
let slide.

I have occasionally waited for Greg to apply a patch and then sent an
email, "Greg has already applied your patch but next time do it like
this." :P

regards,
dan carpenter