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:   Mon, 27 Jul 2020 16:23:51 +0300
From:   Dan Carpenter <dan.carpenter@oracle.com>
To:     Dinghao Liu <dinghao.liu@zju.edu.cn>
Cc:     devel@driverdev.osuosl.org,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>, kjlu@umn.edu,
        linux-kernel@vger.kernel.org, Julia Lawall <julia.lawall@inria.fr>,
        Stefano Brivio <sbrivio@redhat.com>,
        Shreeya Patel <shreeya.patel23498@gmail.com>,
        Larry Finger <Larry.Finger@lwfinger.net>
Subject: Re: [PATCH] Staging: rtl8188eu: rtw_mlme: Fix uninitialized variable
 authmode
Message-ID: <20200727132351.GF1913@kadam>
References: <20200724122957.30411-1-dinghao.liu@zju.edu.cn>
 <20200727114451.GA1913@kadam>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20200727114451.GA1913@kadam>
User-Agent: Mutt/1.9.4 (2018-02-28)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

I review things in the order that they appear in my inbox so I hadn't
seen Greg and Larry's comments.  You've now stumbled into an area of
politics where you have conflicting reviews...  :P  Fortunately, we're
all of us reasonable people.

I think your patch is correct in that it is what the original coder
intended.  You just need to sell the patch correctly in the commit
message.  The real danger of the original code would be if "authmode" is
accidentally 0x30 or 0xdd just because it was uninitialized.  Setting it
to zero ensures that it is not and it also matches how this is handled
in the rtl8723bs driver.  This matches the original author's intention.

So:

1) Re-write the commit message.

    The variable authmode can be uninitialized.  The danger would be
    if it equals _WPA_IE_ID_ (0xdd) or _WPA2_IE_ID_ (0x33).  We can
    avoid this by setting it to zero instead.  This is the approach that
    was used in the rtl8723bs driver.

2) Add a fixes tag.
   Fixes: 7b464c9fa5cc ("staging: r8188eu: Add files for new driver - part 4")

3) Change the commit to Larry's style with the "else if" and "else".
   Sometimes people just disagree about style so it's easiest to go with
   whatever the maintainer (Larry) wants.  It's not worth debating one
   way or the other so just redo it.

Then resend.  Google for "how to send a v2 patch" to get the right
format.

regards,
dan carpenter