mkiiskila commented on a change in pull request #474: PIC32: Fix stack align URL: https://github.com/apache/mynewt-core/pull/474#discussion_r131708815
########## File path: kernel/os/include/os/arch/pic32/os/os_arch.h ########## @@ -38,7 +38,8 @@ typedef uint32_t os_sr_t; /* Stack type, aligned to a 32-bit word. */ #define OS_STACK_PATTERN (0xdeadbeef) -typedef uint32_t os_stack_t; +/* uint64_t in an attempt to get the stack 8 aligned */ +typedef uint64_t os_stack_t; Review comment: My experience with aligned attribute is a bit limited, I usually have used it with specific symbols, not with a type definition. I'm ok if you end up using uint64_t as os_stack_t type, BTW. However, I would recommend making os_arch_task_stack_init() modifications regardless. This because there's no guarantee regarding alignment of malloc()'d stack. With 8 byte alignment of os_stack_t, the number of cases where os_arch_task_stack_init() will get called with unaligned stack should not be many. Usually user would get exactly the stack space they wanted. I'm not too worried about losing 4 bytes here due to alignment, BTW. In practice I think people leave bigger margins for stack; it's a bit tedious to figure the deepest possible call stack, and any code modification/compiler flag change has a chance of changing this for the worse. However, that's just my opinion :). I'm ok with even the current change, and if it turns out not to be enough, we'll modify this further. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
