Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1566205 > unrolled thread

Re: fs/jffs2/readinode.c:189: faulty logic ?

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: fs/jffs2/readinode.c:189: faulty logic ? Richard Weinberger <richard.weinberger@gmail.com> - 2017-01-24 23:30 +0100
    Re: fs/jffs2/readinode.c:189: faulty logic ? Richard Weinberger <richard@nod.at> - 2017-01-25 09:40 +0100

#1566205 — Re: fs/jffs2/readinode.c:189: faulty logic ?

David,

On Tue, Jan 24, 2017 at 9:15 AM, David Binderman <dcb314@hotmail.com> wrote:
> Hello there,
>
> fs/jffs2/readinode.c:189]: (style) Condition 'tn.fn.ofs>=offset' is always true

What tool produces this info?

> Source code is
>
>         if (tn->fn->ofs < offset)
>             next = tn->rb.rb_right;
>         else if (tn->fn->ofs >= offset)
>             next = tn->rb.rb_left;
>         else
>             break;
>
> Maybe better code
>
>         if (tn->fn->ofs < offset)
>             next = tn->rb.rb_right;
>         else if (tn->fn->ofs > offset)
>             next = tn->rb.rb_left;
>         else
>             break;

That code is odd, yes. But we need more info.
Why is fn.fn.ofs>=offset always true?
AFAICT tn->fn->ofs > offset can be also true but
else break is never reached.

-- 
Thanks,
//richard

[toc] | [next] | [standalone]

#1566383

David,

Am 24.01.2017 um 23:57 schrieb David Binderman:
>>> fs/jffs2/readinode.c:189]: (style) Condition 'tn.fn.ofs>=offset' is always true
> 
>>What tool produces this info?
> 
> A static analyser for C and C++ called cppcheck, available from sourceforge.

That needs to go into the changelog.

>>That code is odd, yes. But we need more info.
>>Why is fn.fn.ofs>=offset always true?
> 
> Doh. By obvious inspection of the code.
> If test x < y fails, then x must be >= y.

So tn.fn.ofs>=offset is _not_ always true.
The else branch is always false.

Thanks,
//richard

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web