os: avoid using result of an assignment operator

[DeHess]

Fix coding guideline MISRA C:2012 Rule 13.4 in os:

The result of an assignment operator should not be used.

This PR is part of the enhancement issue https://github.com/zephyrproject-rtos/zephyr/issues/48002 which port the coding guideline fixes done by BUGSENG on the https://github.com/zephyrproject-rtos/zephyr/tree/v2.7-auditable-branch back to main

The commit in this PR is a subset of the original auditable-branch commit: https://github.com/zephyrproject-rtos/zephyr/commit/64336f467c8c4c61f7e926313365e04c8e43f12f

[DeHess]

Converted to for-loops, since this was preferred in other prs.

[dcpleung]

Hm... I think the for loop is even more confusing as you have two assignments there.

[DeHess]

@dcpleung

Hm... I think the for loop is even more confusing as you have two assignments there.

This PR does not necessarily address readability. It addresses the violation of misra coding guideline 13.4:

The result of an assignment operator should not be used

The previous while loop violated this rule, and the for loop I propose to use is the least terrible way to deal with it IMO.

Currently upstream:

while (((fract <<= 1) & BIT_63) == 0) {
    expo--; 
}

Proposed by Bugseng:

for (;;) {
    fract <<= 1;
    if ((fract & BIT_63) != 0) {
        break;
    }
    expo--;
}

Proposed by me:

for (fract <<= 1; (fract & BIT_63) == 0; fract <<= 1) {
    expo--;
}

Here is a discussion about the same topic regarding the same issue in the kernel part of the project: https://github.com/zephyrproject-rtos/zephyr/pull/72226

[andyross]

Yeah, the for() construction was proposed by me in a different PR, because while it's not as clean as the "while(iter = next())" idiom it at least puts the loop invariant at the top of the loop. I really hate the inline break, which is a terrible readability and maintenance cost to pay just for a pedantic rule. The for() duplicates an expression but at least reads naturally.

[andyross]

And is Bugseng actually recommending the infinite loop construct? Yikes.

[dcpleung]

Just curious, had this been run through the tool again to see if it complains?

[DeHess]

@andyross

And is Bugseng actually recommending the infinite loop construct? Yikes.

Yes. For most of these PRs I merely port forward what's on the auditable branch that they left us with.

[DeHess]

@dcpleung

Just curious, had this been run through the tool again to see if it complains?

No, the Tool Bugseng used (Eclair) is proprietary. So unfortunately I don't have access to it and can't check. That being said, my proposed construct is a standard for loop. If it violated the misra guideline, all for loops would.

There are currently efforts underway by @simhein and others to incorporate the Eclair Tool into the CI Pipeline in some way or other. My PRs are supposed to pave the way, so that once we use the tool to go over the entire project again, most violations will already be fixed.