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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> 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


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