Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
From:   Kalle Valo <kvalo@kernel.org>
To:     Jeff Johnson <quic_jjohnson@quicinc.com>
Cc:     <ath11k@lists.infradead.org>, <linux-wireless@vger.kernel.org>
Subject: Re: [PATCH 1/2] ath11k: mac: fix too long line
References: <20220503060415.24499-1-kvalo@kernel.org>
        <f2f27831-03eb-3788-5855-649c0f33c785@quicinc.com>
        <87y1zhalfy.fsf@kernel.org>
        <1615688c-fb7d-261a-7b01-fe47c74b9597@quicinc.com>
Date:   Fri, 06 May 2022 12:04:42 +0300
In-Reply-To: <1615688c-fb7d-261a-7b01-fe47c74b9597@quicinc.com> (Jeff
        Johnson's message of "Wed, 4 May 2022 09:30:35 -0700")
Message-ID: <87y1zf821h.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

Jeff Johnson <quic_jjohnson@quicinc.com> writes:

> On 5/3/2022 10:58 PM, Kalle Valo wrote:
>> Jeff Johnson <quic_jjohnson@quicinc.com> writes:
> pe>> is there a reason to not declare deflink here?
>>> then its scope of definition would equal the scope of usage
>>
>> In ath10k and ath11k I have tried to avoid that and instead declare all
>> variables in the beginning of the function, this is to keep the code
>> simple. Of course there are few cases where a variable is declared in
>> the middle of the function, but that's just sloppy review on my part. I
>> feel that it's better to refactor the function into smaller functions
>> than start declaring variables in the middle of functions. Does that
>> make sense?
>>
>
> This is really an academic question.
>
> In the larger kernel community I'm seeing a push to reduce the scope
> of identifiers in some cases:
> 1) declaring loop control variables within the actual loop control operation
> 2) prohibiting the access to list iterators outside the list iteration loop

Yeah, those are definitely good examples when to limit scope.

And of course this isn't a hard rule, in some cases we have to make
exceptions. For example ath11k_mac_op_bss_info_changed() and
ath11k_mac_op_sta_state() are long functions but trying to split them to
smaller functions most likely makes them harder to read. So for these
functions having variables declared within if blocks would sound like a
good idea, I think it would make the functions more readable.

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

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