From:   Kalle Valo <kvalo@kernel.org>
To:     Marc Bornand <dev.mbornand@systemb.ch>
Cc:     Johannes Berg <johannes@sipsolutions.net>,
        linux-wireless@vger.kernel.org,
        "David S . Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>, netdev@vger.kernel.org,
        linux-kernel@vger.kernel.org, Yohan Prod'homme <kernel@zoddo.fr>,
        stable@vger.kernel.org
Subject: Re: [PATCH v4] Set ssid when authenticating
References: <20230214132009.1011452-1-dev.mbornand@systemb.ch>
        <87ttzn4hki.fsf@kernel.org> <Y+yT2YUORRHY4bei@opmb2>
Date:   Wed, 15 Feb 2023 10:32:18 +0200
In-Reply-To: <Y+yT2YUORRHY4bei@opmb2> (Marc Bornand's message of "Wed, 15 Feb
        2023 08:12:17 +0000")
Message-ID: <87lekz49d9.fsf@kernel.org>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain
Precedence: bulk

Marc Bornand <dev.mbornand@systemb.ch> writes:

> On Wed, Feb 15, 2023 at 07:35:09AM +0200, Kalle Valo wrote:
>> Marc Bornand <dev.mbornand@systemb.ch> writes:
>>
>> > changes since v3:
>> > - add missing NULL check
>> > - add missing break
>> >
>> > changes since v2:
>> > - The code was tottaly rewritten based on the disscution of the
>> >   v2 patch.
>> > - the ssid is set in __cfg80211_connect_result() and only if the ssid is
>> >   not already set.
>> > - Do not add an other ssid reset path since it is already done in
>> >   __cfg80211_disconnected()
>> >
>> > When a connexion was established without going through
>> > NL80211_CMD_CONNECT, the ssid was never set in the wireless_dev struct.
>> > Now we set it in __cfg80211_connect_result() when it is not already set.
>> >
>> > Reported-by: Yohan Prod'homme <kernel@zoddo.fr>
>> > Fixes: 7b0a0e3c3a88260b6fcb017e49f198463aa62ed1
>> > Cc: linux-wireless@vger.kernel.org
>> > Cc: stable@vger.kernel.org
>> > Link: https://bugzilla.kernel.org/show_bug.cgi?id=216711
>> > Signed-off-by: Marc Bornand <dev.mbornand@systemb.ch>
>> > ---
>> >  net/wireless/sme.c | 17 +++++++++++++++++
>> >  1 file changed, 17 insertions(+)
>>
>> The change log ("changes since v3" etc) should be after "---" line and
>
> Does it need another "---" after the change log?
> something like:
>
> "---"
> "changes since v3:"
> "(CHANGES)"
> "---"

No need to add a second "---" line.

>> the title should start with "wifi: cfg80211:". Please read the wiki link
>> below.
>
> Should i start with the version 1 with the new title?

If you reset the version number that might confuse the reviewers, so my
recommendation is to use v5 in the next version. That makes it more
obvious that there are earlier versions available.

> and since i am already changing the title, the following might better
> discribe the patch, or should i keep the old title after the ":" ?
>
> [PATCH wireless] wifi: cfg80211: Set SSID if it is not already set

Changing the title is fine.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches