Code review comment for lp:~vlad-lesin/percona-server/5.5-bug-1351148

Revision history for this message
Laurynas Biveinis (laurynas-biveinis) wrote :

    - I don't understand the stack_ends_here calculation for the
      #else case. In the first line we round down &tmp to the page
      size, and then we add either 0, either STACK_DIRECTION * stack
      size. Adding zero if STACK_DIRECTION == 1 does not look right?
      It seems the intention was to jump one page if the stack grows
      down, but for that the evaluation precedence is wrong, it needs
      extra parentheses around the ?: operator.

    - Related to the above, there is a warning regression with clang 6.0:

/Users/laurynas/percona/lp-mysql-server/5.5-bug-1351148/mysys/my_thr_init.c:371:58: warning: operator '?:' has lower precedence than '+'; '+' will be evaluated first [-Wparentheses]
                                  (STACK_DIRECTION == 1) ? 0 : my_getpagesize() +
                                  ~~~~~~~~~~~~~~~~~~~~~~ ^
/Users/laurynas/percona/lp-mysql-server/5.5-bug-1351148/mysys/my_thr_init.c:371:58: note: place parentheses around the '+' expression to silence this warning
                                  (STACK_DIRECTION == 1) ? 0 : my_getpagesize() +
                                                         ^
/Users/laurynas/percona/lp-mysql-server/5.5-bug-1351148/mysys/my_thr_init.c:371:58: note: place parentheses around the '?:' expression to evaluate it first
                                  (STACK_DIRECTION == 1) ? 0 : my_getpagesize() +

review: Needs Fixing

« Back to merge proposal