[Bug 37482] New: Suspicious condition (assignment + comparison); Clarify expression with parentheses. (Cppcheck)

https://bugs.winehq.org/show_bug.cgi?id=37482

Bug ID: 37482
           Summary: Suspicious condition (assignment + comparison);
                    Clarify expression with parentheses. (Cppcheck)
           Product: Wine
           Version: 1.7.29
          Hardware: x86
                OS: Linux
            Status: NEW
          Severity: normal
          Priority: P2
         Component: comctl32
          Assignee: [e-mail]
          Reporter: [e-mail]
                CC: [e-mail]
      Distribution: ---

[dlls\comctl32\listview.c:5056]: (style) Suspicious condition (assignment +
comparison); Clarify expression with parentheses.

5052         /* draw the horizontal lines for the rows */
 5053         itemheight =  LISTVIEW_CalculateItemHeight(infoPtr);
 5054         rcItem.left   = infoPtr->rcList.left;
 5055         rcItem.right  = infoPtr->rcList.right;
 5056         for(y = Origin.y > 1 ? Origin.y - 1 : itemheight - 1 + Origin.y %
itemheight; y<=infoPtr->rcList.bot      tom; y+=itemheight)
 5057         {
 5058             rcItem.bottom = rcItem.top = y;
 5059             TRACE("horz rcItem=%s\n", wine_dbgstr_rect(&rcItem));
 5060             MoveToEx (hdc, rcItem.left, rcItem.top, NULL);
 5061             LineTo (hdc, rcItem.right, rcItem.top);
 5062         }

[Bug 37482] Suspicious condition (assignment + comparison); Clarify expression with parentheses. (Cppcheck)

https://bugs.winehq.org/show_bug.cgi?id=37482

--- Comment #1 from Michael Stefaniuc  ---
I would say clang is misguided here. Not sure why it complains here as
    y = comparison ? foo : bar
is the standard way of doing this and adding paranthesis would be bad style.

Can you please report it to them as false positive?

[Bug 37482] Suspicious condition (assignment + comparison); Clarify expression with parentheses. (Cppcheck)

https://bugs.winehq.org/show_bug.cgi?id=37482

--- Comment #2 from Bruno Jesus  ---
(In reply to Michael Stefaniuc from comment #1)
> I would say clang is misguided here. Not sure why it complains here as
>     y = comparison ? foo : bar
> is the standard way of doing this and adding paranthesis would be bad style.
>
> Can you please report it to them as false positive?

Ok, I think the program was misguided because the false branch is very long,
thinking that some compiler could do things like:

y = (Origin.y > 1 ? Origin.y - 1 : itemheight) - 1 + Origin.y % itemheight
y = (Origin.y > 1 ? Origin.y - 1 : itemheight - 1) + Origin.y % itemheight
y = (Origin.y > 1 ? Origin.y - 1 : itemheight - 1 + Origin.y) % itemheight
y = (Origin.y > 1 ? Origin.y - 1 : itemheight - 1 + Origin.y % itemheight)

[Bug 37482] Suspicious condition (assignment + comparison); Clarify expression with parentheses. (Cppcheck)

https://bugs.winehq.org/show_bug.cgi?id=37482

Bruno Jesus  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |UPSTREAM

--- Comment #3 from Bruno Jesus  ---
Upstream bug opened.
http://trac.cppcheck.net/ticket/6249

[Bug 37482] Suspicious condition (assignment + comparison); Clarify expression with parentheses. (Cppcheck)

https://bugs.winehq.org/show_bug.cgi?id=37482

--- Comment #4 from Nikolay Sivov  ---
Looks like it's already fixed with
http://trac.cppcheck.net/changeset/eec938dbad86cefaa51cc65e901616f5fa477ee7/,
that should be included in 1.69 release.

[Bug 37482] Suspicious condition (assignment + comparison); Clarify expression with parentheses. (Cppcheck)

https://bugs.winehq.org/show_bug.cgi?id=37482

Bruno Jesus  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |CLOSED

--- Comment #5 from Bruno Jesus  ---
(In reply to Nikolay Sivov from comment #4)
> Looks like it's already fixed with
> http://trac.cppcheck.net/changeset/eec938dbad86cefaa51cc65e901616f5fa477ee7/,
> that should be included in 1.69 release.

Closing upstream, thanks for taking a look at this.