Re: [Xen-devel] [PATCH for-4.9] x86/mm: Placate DEADCODE Coverity warning

[Julien Grall <julien.grall@xxxxxxx>]

CC Ian + Wei for the testing

On 01/06/17 12:21, Jan Beulich wrote:
> > > > On 01.06.17 at 13:18, <julien.grall@xxxxxxx> wrote:
> > Hi Jan,
> >
> > On 01/06/17 12:15, Jan Beulich wrote:
> > > > > > On 01.06.17 at 13:09, <julien.grall@xxxxxxx> wrote:
> > > > Hi Andrew,
> > > >
> > > > On 31/05/17 14:23, Andrew Cooper wrote:
> > > > > On 31/05/17 09:52, Julien Grall wrote:
> > > > > > Hi,
> > > > > >
> > > > > > On 05/22/2017 02:32 PM, Jan Beulich wrote:
> > > > > > > > > > On 22.05.17 at 15:12, <andrew.cooper3@xxxxxxxxxx> wrote:
> > > > > > > > _PAGE_GNTTAB is only used in debug builds of Xen; in release builds,
> > > > > > > > it has
> > > > > > > > the value 0.  Coverity complains that "l1e_get_flags(l1e) & 0" is
> > > > > > > > logically
> > > > > > > > dead.
> > > > > > > >
> > > > > > > > Add an extra condition into the logic to skip the flag check if
> > > > > > > > _PAGE_GNTTAB
> > > > > > > > is 0.
> > > > > > > And this helps? To me "if ( 0 && ... )" and "if ( (x & 0) && ... )" look
> > > > > > > logically the same (i.e. I'd expect the same warnings to be triggered
> > > > > > > [or not]).
> > > > > > I haven't seen any answer on this question. Andrew, does this patch
> > > > > > still hold for Xen 4.9?
> > > > > Sorry - it fell through the cracks, but yes, it does stand for 4.9.
> > > > >
> > > > > As to the "if ( 0 && ... )" and "if ( (x & 0) && ... )", one is very
> > > > > clearly a "short circuit every thing else if this value is zero", while
> > > > > the other looks like a programming mistake, which is also why I expect
> > > > > this to resolve Coverity's complaint.
> > > > >
> > > > > Unfortunately, I can't be certain that this will resolve the issue until
> > > > > it gets committed, as I don't have a useful way to run Coverity on
> > > > > arbitrary non-debug builds.
> > > > Are we running staging-4.* branch on Coverity? Looking at the git, I see
> > > > *coverity* branch only on unstable.
> > > Yes, so my suggestion would be to commit the patch on master, see
> > > if it helps, and if so consider backporting for 4.9. If it doesn't help,
> > > it should be reverted or replaced by something "better".
> > Well master has now debug enabled and AFAIU the warning can only
> > occurred on non-debug build. So I am not sure how this would help to be
> > in master.
> Oh, good point. I think it would really be a good idea to have
> both debug and non-debug builds tested, which at once would
> avoid sudden bursts of issues when switching the default.
>
> Jan
--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel