| Message ID | 20200914191925.24192-1-alex.dewar90@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ath10k: sdio: remove reduntant check in for loop | expand |
On Mon, 2020-09-14 at 20:19 +0100, Alex Dewar wrote: > The for loop checks whether cur_section is NULL on every iteration, > but > we know it can never be NULL as there is another check towards the > bottom of the loop body. Remove this unnecessary check. > > Also change i to start at 1, so that we don't need an extra +1 wheno > we > use it. > > Addresses-Coverity: 1496984 ("Null pointer dereferences) > Signed-off-by: Alex Dewar <alex.dewar90@gmail.com> > --- > drivers/net/wireless/ath/ath10k/sdio.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/wireless/ath/ath10k/sdio.c > b/drivers/net/wireless/ath/ath10k/sdio.c > index 81ddaafb6721..f31ab2ec2c48 100644 > --- a/drivers/net/wireless/ath/ath10k/sdio.c > +++ b/drivers/net/wireless/ath/ath10k/sdio.c > @@ -2308,7 +2308,7 @@ static int > ath10k_sdio_dump_memory_section(struct ath10k *ar, > > count = 0; > > - for (i = 0; cur_section; i++) { > + for (i = 1; ; i++) { '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 == ...) > + if (i == mem_region->section_table.size) { > /* last section */ > next_section = NULL; > skip_size = 0;
[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;
diff --git a/drivers/net/wireless/ath/ath10k/sdio.c b/drivers/net/wireless/ath/ath10k/sdio.c index 81ddaafb6721..f31ab2ec2c48 100644 --- a/drivers/net/wireless/ath/ath10k/sdio.c +++ b/drivers/net/wireless/ath/ath10k/sdio.c @@ -2308,7 +2308,7 @@ static int ath10k_sdio_dump_memory_section(struct ath10k *ar, count = 0; - for (i = 0; cur_section; i++) { + for (i = 1; ; i++) { 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) { + if (i == mem_region->section_table.size) { /* last section */ next_section = NULL; skip_size = 0;
The for loop checks whether cur_section is NULL on every iteration, but we know it can never be NULL as there is another check towards the bottom of the loop body. Remove this unnecessary check. Also change i to start at 1, so that we don't need an extra +1 when we use it. Addresses-Coverity: 1496984 ("Null pointer dereferences) Signed-off-by: Alex Dewar <alex.dewar90@gmail.com> --- drivers/net/wireless/ath/ath10k/sdio.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)