[GitHub] [incubator-nuttx] johannes-nivus commented on a change in pull request #1900: ARM stack fix: Same boundary calculation in do_stackcheck and stack_color.

[GitBox <gi...@apache.org>]

johannes-nivus commented on a change in pull request #1900:
URL: https://github.com/apache/incubator-nuttx/pull/1900#discussion_r495570447



##########
File path: arch/arm/src/common/arm_createstack.c
##########
@@ -237,19 +246,27 @@ int up_create_stack(FAR struct tcb_s *tcb, size_t stack_size, uint8_t ttype)
  ****************************************************************************/
 
 #ifdef CONFIG_STACK_COLORATION
-void arm_stack_color(FAR void *stackbase, size_t nbytes)
+void arm_stack_color(FAR void *stackbase, size_t size)
 {
-  /* Take extra care that we do not write outsize the stack boundaries */
+  uintptr_t start;
+  uintptr_t end;
+  FAR uint32_t *ptr;
+
+  /* Take extra care that we do not write outside the stack boundaries */
+
+  start = INT32_ALIGN_UP((uintptr_t)stackbase);
+  end   = INT32_ALIGN_DOWN((uintptr_t)stackbase + size);
+
+  /* Get the adjusted size based on the top and bottom of the stack */
 
-  uint32_t *stkptr = (uint32_t *)(((uintptr_t)stackbase + 3) & ~3);
-  uintptr_t stkend = (((uintptr_t)stackbase + nbytes) & ~3);
-  size_t    nwords = (stkend - (uintptr_t)stackbase) >> 2;
+  size = (end - start) >> 2;

Review comment:
       You're right, I'll change the naming.
   And yes, all the calculations are unneccesary since createstack and usestack already align the stacks properly. I wasn't sure about removing it all together... in stack_color and do_stackcheck.
   I'd be fine with a debugassert as well. @davids5 what do you think?
   




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org