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;
Subject: Re: [PATCH] ath10k: sdio: remove reduntant check in for loop
To:     Saeed Mahameed <saeedm@nvidia.com>
Cc:     "linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>,
        "ath10k@lists.infradead.org" <ath10k@lists.infradead.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "kvalo@codeaurora.org" <kvalo@codeaurora.org>,
        "davem@davemloft.net" <davem@davemloft.net>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        "kuba@kernel.org" <kuba@kernel.org>
References: <20200914191925.24192-1-alex.dewar90@gmail.com>
 <c2987351e3bdad16510dd35847991c2412a9db6b.camel@nvidia.com>
From:   Alex Dewar <alex.dewar90@gmail.com>
Message-ID: <57efff54-7aa4-8220-c705-1fdf35b0099e@gmail.com>
Date:   Wed, 16 Sep 2020 17:59:11 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.12.0
MIME-Version: 1.0
In-Reply-To: <c2987351e3bdad16510dd35847991c2412a9db6b.camel@nvidia.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 7bit
Content-Language: en-US
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

[snip]
> 'i' is only referenced once inside the loop to check boundary,
>
> the loop is actually iterating over cur_section, so i would make it
> clear in the loop statement, e.g.:
> Remove the break condition and the cur_section assignment at the end of
> the loop and use the loop statement to do it for you
>
> for (; cur_section; cur_section = next_section)
>
>
>>   		section_size = cur_section->end - cur_section->start;
>>   
>>   		if (section_size <= 0) {
>> @@ -2318,7 +2318,7 @@ static int
>> ath10k_sdio_dump_memory_section(struct ath10k *ar,
>>   			break;
>>   		}
>>   
>> -		if ((i + 1) == mem_region->section_table.size) {
> And for i you can just increment it inline:
> if (++i == ...)

Good suggestions! I've sent a v2 with these changes.

>      
>
>> +		if (i == mem_region->section_table.size) {
>>   			/* last section */
>>   			next_section = NULL;
>>   			skip_size = 0;