ARMmbed / core-util

DEPRECATED: Mbed 3 utilities library
Other
12 stars 17 forks source link

add a level of indirection in accessing mbed_sbrk_start (and similar)… #28

Closed rgrover closed 8 years ago

rgrover commented 8 years ago

… to support POSIX

@bogdanm @0xc0170 @bremoran

rgrover commented 8 years ago

tests for mbed-drivers pass (using frdm-k64f-gcc)

bremoran commented 8 years ago

Other than the style guidelines, looks good.

0xc0170 commented 8 years ago

Apart from above raised by Brendan, LGTM

rgrover commented 8 years ago

@0xc0170 https://developer.mbed.org/teams/SDK-Development/wiki/mbed-sdk-coding-style requires that preprocessor macros start at the beginning of a new line; should that rule also apply in the case of nesting?

bremoran commented 8 years ago

The rule provided in the style guide has no exceptions. Perhaps we should create a RFC for changes to the style guide. Until that's been done, the style guide is what we have.

0xc0170 commented 8 years ago

I am planning to create that RFC, we need update! Will do soon.

What we can find in our sorces are these 2 (+/-):

#if !defined(MBED_SBRK_START) || !defined(MBED_KRBS_START)
#   ifndef TARGET_LIKE_POSIX
#       ifdef __ARMCC_VERSION
#           define __mbed_sbrk_start (Image$$ARM_LIB_HEAP$$Base)
#           define __mbed_krbs_start (Image$$ARM_LIB_HEAP$$ZI$$Limit)
#           pragma import(__use_two_region_memory)
#       endif /*#ifdef __ARMCC_VERSION */
extern unsigned int __mbed_sbrk_start;
extern unsigned int __mbed_krbs_start;
#       define MBED_SBRK_START &__mbed_sbrk_start
#       define MBED_KRBS_START &__mbed_krbs_start
#   else /* #ifdef TARGET_LIKE_POSIX */
extern void *__mbed_sbrk_start;
extern void *__mbed_krbs_start;
#       define MBED_SBRK_START __mbed_sbrk_start
#       define MBED_KRBS_START __mbed_krbs_start
#   endif /* #ifdef TARGET_LIKE_POSIX */
#endif

vs

#if !defined(MBED_SBRK_START) || !defined(MBED_KRBS_START)

#ifndef TARGET_LIKE_POSIX

#ifdef __ARMCC_VERSION
#define __mbed_sbrk_start (Image$$ARM_LIB_HEAP$$Base)
#define __mbed_krbs_start (Image$$ARM_LIB_HEAP$$ZI$$Limit)
#pragma import(__use_two_region_memory)
#endif /*#ifdef __ARMCC_VERSION */

extern unsigned int __mbed_sbrk_start;
extern unsigned int __mbed_krbs_start;
#define MBED_SBRK_START &__mbed_sbrk_start
#define MBED_KRBS_START &__mbed_krbs_start

#else /* #ifdef TARGET_LIKE_POSIX */
extern void *__mbed_sbrk_start;
extern void *__mbed_krbs_start;
#define MBED_SBRK_START __mbed_sbrk_start
#define MBED_KRBS_START __mbed_krbs_start
#endif /* #ifdef TARGET_LIKE_POSIX */
#endif
rgrover commented 8 years ago

I like the first of the above, but perhaps the '#' need not dangle on its own at column 1.

rgrover commented 8 years ago

@0xc0170 @bremoran please review the white space diffs around indentation of nested preprocessor statements. To me it looks harder to read now, but it conforms with our standard.

rgrover commented 8 years ago

@0xc0170

bremoran commented 8 years ago

@rgrover While I don't think that's the intention of the coding style guide, it's sure a lot more readable, so +1 for me!

0xc0170 commented 8 years ago

@rgrover +1