Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixing -Wmisleading-indentation #90

Closed
AKuHAK opened this issue Jan 19, 2019 · 4 comments
Closed

Fixing -Wmisleading-indentation #90

AKuHAK opened this issue Jan 19, 2019 · 4 comments

Comments

@AKuHAK
Copy link
Contributor

AKuHAK commented Jan 19, 2019

Just reopen #31 cause I fixed in #88 only part of warnings. There is still some of them:

src/callstack.c: In function 'ps2GetStackTrace':
src/callstack.c:200:14: warning: this 'if' clause does not guard... [-Wmisleading-indentation]
         else if (inst == RETURN)
              ^~
src/callstack.c:202:11: note: ...this statement, but the latter is misleadingly indented as if it were guarded by the 'if'
           ra++;
           ^~
src/callstack.c:205:7: warning: this 'if' clause does not guard... [-Wmisleading-indentation]
       if (sp_adjust == 0 && (found_const_upper || found_const_lower))
       ^~
src/callstack.c:207:9: note: ...this statement, but the latter is misleadingly indented as if it were guarded by the 'if'
         rc->raOffset = ra_offset;
         ^~
src/stdtrf.c: In function 'stdtrf':
src/stdtrf.c:155:1: warning: this 'if' clause does not guard... [-Wmisleading-indentation]
 if( t < 0 )
 ^~
src/stdtrf.c:158:2: note: ...this statement, but the latter is misleadingly indented as if it were guarded by the 'if'
  p = 0.5 + 0.5 * p;
  ^

Im not completely sure if this is real missing brackets or just indentation problem. For example first warning can bi fixed in such way:

      while ((!found_ra_offset || !found_sp_adjust) && ra < ra_limit)
      {
        inst = *ra;
        /* look for the offset of the PC in the stack frame */
        if ((inst & RESTORE_RETURNVAL_MASK) == RESTORE_RETURNVAL)
        {
          ra_offset = inst & ~RESTORE_RETURNVAL_MASK;
          found_ra_offset = TRUE;
        }
        else if ((inst & RESTORE_RETURNVAL_MASK) == RESTORE_RETURNVAL2)
        {
            ra_offset = inst & ~RESTORE_RETURNVAL_MASK;
            found_ra_offset = TRUE;
        }
        else if ((inst & RESTORE_RETURNVAL_MASK) == RESTORE_RETURNVAL3)
        {
            ra_offset = inst & ~RESTORE_RETURNVAL_MASK;
            found_ra_offset = TRUE;
        }
        else if ((inst & ADJUST_STACKP_C_MASK) == ADJUST_STACKP_C)
        {
          sp_adjust = inst & ~ADJUST_STACKP_C_MASK;
          found_sp_adjust = TRUE;
        }
        else if ((inst & ADJUST_STACKP_V_MASK) == ADJUST_STACKP_V)
        {
          sp_adjust = 0;
          found_sp_adjust = TRUE;
        }
        else if ((inst & SET_UPPER_C_MASK) == SET_UPPER_C)
        {
          const_upper = inst & ~SET_UPPER_C_MASK;
          const_lower = 0;
          found_const_upper = TRUE;
        }
        else if ((inst & OR_LOWER_C_MASK) == OR_LOWER_C)
        {
          const_lower = inst & ~OR_LOWER_C_MASK;
          found_const_lower = TRUE;
        }
        else if ((inst & SET_LOWER_C_MASK) == SET_LOWER_C)
        {
          const_lower = inst & ~SET_LOWER_C_MASK;
          const_upper = 0;
          found_const_lower = TRUE;
        }
        else if (inst == RETURN)
        {
          ra_limit = ra + 2;
          ra++;
        }
      }

or in such way:

      while ((!found_ra_offset || !found_sp_adjust) && ra < ra_limit)
      {
        inst = *ra;
        /* look for the offset of the PC in the stack frame */
        if ((inst & RESTORE_RETURNVAL_MASK) == RESTORE_RETURNVAL)
        {
          ra_offset = inst & ~RESTORE_RETURNVAL_MASK;
          found_ra_offset = TRUE;
        }
        else if ((inst & RESTORE_RETURNVAL_MASK) == RESTORE_RETURNVAL2)
        {
            ra_offset = inst & ~RESTORE_RETURNVAL_MASK;
            found_ra_offset = TRUE;
        }
        else if ((inst & RESTORE_RETURNVAL_MASK) == RESTORE_RETURNVAL3)
        {
            ra_offset = inst & ~RESTORE_RETURNVAL_MASK;
            found_ra_offset = TRUE;
        }
        else if ((inst & ADJUST_STACKP_C_MASK) == ADJUST_STACKP_C)
        {
          sp_adjust = inst & ~ADJUST_STACKP_C_MASK;
          found_sp_adjust = TRUE;
        }
        else if ((inst & ADJUST_STACKP_V_MASK) == ADJUST_STACKP_V)
        {
          sp_adjust = 0;
          found_sp_adjust = TRUE;
        }
        else if ((inst & SET_UPPER_C_MASK) == SET_UPPER_C)
        {
          const_upper = inst & ~SET_UPPER_C_MASK;
          const_lower = 0;
          found_const_upper = TRUE;
        }
        else if ((inst & OR_LOWER_C_MASK) == OR_LOWER_C)
        {
          const_lower = inst & ~OR_LOWER_C_MASK;
          found_const_lower = TRUE;
        }
        else if ((inst & SET_LOWER_C_MASK) == SET_LOWER_C)
        {
          const_lower = inst & ~SET_LOWER_C_MASK;
          const_upper = 0;
          found_const_lower = TRUE;
        }
        else if (inst == RETURN)
          ra_limit = ra + 2;
        ra++;
      }

And this code isnt that easy so I will left it here - maybe somebody with better skills can dig in it.

@TnA-Plastic
Copy link

TnA-Plastic commented Jan 19, 2019

It's a bit lot of 'else if's... Maybe a 'switch' or similar would be better suited to shorten the code (and possibly making it more efficient)?!

Edit: Well, I am not sure if it would work. I just skimmed over it and realized there are quite few similar vars to compare with... Hm...

@sp193
Copy link
Member

sp193 commented Jan 19, 2019

We cannot use a switch-case block with bitwise operations.

@TnA-Plastic
Copy link

Yes... I was too fast on posting and edited my post.

I still think there could be an alternative/shorter code.

But it probably doesn't matter much anyway.

@sp193
Copy link
Member

sp193 commented Jan 25, 2019

My own thoughts about these:

  • ee/debug/src/callstack.c:
    • We should decrease identation of line 202 by 1. ra++ should be a post-loop thing, otherwise it will hang.
    • As for 205-207, it gets tricky. But if we assume that the code is presently working, then we can just fix the identation by decreasing the identation of lines 207-231 by 1 level.
  • ee/math/src/stdtrf.c: decrease identation of line 158 by 1, if we assume the math is presently working.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants