linux-kernel - Re: drivers/dma/pl330.c:2981:9: warning: Identical condition 'ret', second condition is always false

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Mon, 24 Aug 2020 19:24:38 +0530
From:   Vinod Koul <vkoul@...nel.org>
To:     Marek Szyprowski <m.szyprowski@...sung.com>
Cc:     kernel test robot <lkp@...el.com>,
        Sugar Zhang <sugar.zhang@...k-chips.com>,
        Ulf Hansson <ulf.hansson@...aro.org>, kbuild-all@...ts.01.org,
        linux-kernel@...r.kernel.org
Subject: Re: drivers/dma/pl330.c:2981:9: warning: Identical condition 'ret',
 second condition is always false

On 24-08-20, 15:50, Marek Szyprowski wrote:
> Hi Vinod,
> 
> On 08.08.2020 14:59, Vinod Koul wrote:
> > On 08-08-20, 10:53, kernel test robot wrote:
> >> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> >> head:   30185b69a2d533c4ba6ca926b8390ce7de495e29
> >> commit: a39cddc9e3775100100a4272feed64faac831be9 dmaengine: pl330: Drop boilerplate code for suspend/resume
> >> date:   8 months ago
> >> compiler: aarch64-linux-gcc (GCC) 9.3.0
> >>
> >> If you fix the issue, kindly add following tag as appropriate
> >> Reported-by: kernel test robot <lkp@...el.com>
> >>
> >>
> >> cppcheck warnings: (new ones prefixed by >>)
> >>
> >>>> drivers/dma/pl330.c:2981:9: warning: Identical condition 'ret', second condition is always false [identicalConditionAfterEarlyExit]
> >>      return ret;
> >>             ^
> >>     drivers/dma/pl330.c:2976:6: note: first condition
> >>      if (ret)
> >>          ^
> >>     drivers/dma/pl330.c:2981:9: note: second condition
> >>      return ret;
> > This one could be fixed by making this as return 0, but is harmless
> 
> Right, with CONFIG_PM disabled, pm_runtime_force_resume() is noop, what 
> causes the above warning.
> 
> >>             ^
> >>>> drivers/dma/pl330.c:2798:23: warning: Variable 'desc->rqcfg.brst_len' is reassigned a value before the old one has been used. [redundantAssignment]
> >>      desc->rqcfg.brst_len = get_burst_len(desc, len);
> >>                           ^
> >>     drivers/dma/pl330.c:2796:24: note: Variable 'desc->rqcfg.brst_len' is reassigned a value before the old one has been used.
> >>       desc->rqcfg.brst_len = 1;
> >>                            ^
> >>     drivers/dma/pl330.c:2798:23: note: Variable 'desc->rqcfg.brst_len' is reassigned a value before the old one has been used.
> >>      desc->rqcfg.brst_len = get_burst_len(desc, len);
> > This one actually seems like a bug. Reading the code I think
> > get_burst_len() should be called first before checking if burst size is
> > smaller and setting to 1 in that case
> >
> > Sugar Zhang, Marek Szyprowski can you folks check this?
> 
> Indeed. It look that the commit 137bd11090d89b added 
> desc->rqcfg.brst_len = 1 assignment before the desc->rqcfg.brst_len is 
> set. Maybe this was a result of the broken rebase or so. No idea. It 
> makes sense to switch the order and call desc->rqcfg.brst_len = 
> get_burst_len(desc, len) first. I can send a patch if you want.

That would be great Marek

Thanks
-- 
~Vinod