Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Thu, 1 Apr 2021 12:50:18 +0300
From:   Dan Carpenter <dan.carpenter@oracle.com>
To:     Fabio Aiuto <fabioaiuto83@gmail.com>
Cc:     gregkh@linuxfoundation.org, joe@perches.com,
        linux-staging@lists.linux.dev, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 08/49] staging: rtl8723bs: remove RT_TRACE logs in
 core/rtw_cmd.c
Message-ID: <20210401095017.GR2065@kadam>
References: <cover.1617268327.git.fabioaiuto83@gmail.com>
 <7f51432d99459d79742639341f107115f0c224c5.1617268327.git.fabioaiuto83@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <7f51432d99459d79742639341f107115f0c224c5.1617268327.git.fabioaiuto83@gmail.com>
User-Agent: Mutt/1.9.4 (2018-02-28)
Precedence: bulk

On Thu, Apr 01, 2021 at 11:20:38AM +0200, Fabio Aiuto wrote:
> @@ -677,9 +663,8 @@ u8 rtw_createbss_cmd(struct adapter  *padapter)
>  	u8 res = _SUCCESS;
>  
>  	if (pmlmepriv->assoc_ssid.SsidLength == 0)
> -		RT_TRACE(_module_rtl871x_cmd_c_, _drv_info_, (" createbss for Any SSid:%s\n", pmlmepriv->assoc_ssid.Ssid));
> +		;
>  	else
> -		RT_TRACE(_module_rtl871x_cmd_c_, _drv_info_, (" createbss for SSid:%s\n", pmlmepriv->assoc_ssid.Ssid));
>  
>  	pcmd = rtw_zmalloc(sizeof(struct cmd_obj));

This is a bug.  Smatch has a check for this which hopefully would have
detected it (I haven't tested).

There are some more similar issues below as well.  So generally the rule
is don't adjust the indenting if it's not related to your patch.  In
some cases you have been fixing the indenting but it should be done in
a separate patch.  But the other rule is that if your patch introduces
a checkpatch warning then you need to fix it in the same patch.  In
this block the whole if statement should be removed.  But also if you
have something like:

	if (foo) {
		RT_TRACE(blha blah blah);
		return;
	}

Then checkpatch will complain that the the curly braces are not
required.  (Checkpatch might not complain for your patch but it will
complain when we re-run it with the -f option over the whole file).  So
you should update this to:

	if (foo)
		return;

That's all considered part of deleting the RT_TRACE().  Also if there
are empty curly braces then delete those in the same patch.

I have looked over patches 1-7 and those seem basically fine.  I'm not
going to review any further into this patchset because you're going to
have to redo them and I will be reviewing the v2 set later anyway.  So
just look it over yourself and check for any similar issues.

regards,
dan carpenter