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

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

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



##########
File path: arch/arm/src/common/arm_checkstack.c
##########
@@ -78,10 +88,10 @@ static size_t do_stackcheck(uintptr_t alloc, size_t size);
  *
  ****************************************************************************/
 
-static size_t do_stackcheck(uintptr_t alloc, size_t size)
+static size_t do_stackcheck(FAR void *stackbase, size_t size)

Review comment:
       Wouldn't the allocation have resulted in a properly aligned (adjusted base pointer) and adjusted size? Both should be  properly aligned: Such that (base + size ) & 7 == 0 && (size & 3) == 0. So no adjusting should be needed and in checking, and in fact if the the passed parameters do not meet this test there has been TCB corruption.

##########
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)

Review comment:
       By changing the name, we are loosing information. It would be best to leave it the same or document the parameters to all the functions.




----------------------------------------------------------------
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