-
Notifications
You must be signed in to change notification settings - Fork 502
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Change or remove include/Platforms/c2000/stdint.h
#1792
Comments
I've never been able to get a C2000 build stood up so I've been hesitant to touch it. If you're able to test the change, a pull request would be appreciated. |
I can try, but I'm not sure where to start.
I try to figure out how to test it in a few weeks and will keep you updated on the available options before creating a PR. Is there a guideline/strategy I can fallow on how to deal with deprecated and/or non-testable platforms in this project (upgrade to newer version, drop support, etc.)? |
We tend to try to keep support for old platforms and versions. So, if it is possible to put the newer version next to the existing one then often that is preferred (so people using old versions can also use it). If that becomes too troublesome, then better just upgrade. |
The named file defines
uint8_t
for C2000 MCUs using the following line:cpputest/include/Platforms/c2000/stdint.h
Line 34 in 81eb8b8
This is done because on C2000 platforms, a
char
is 16bit (as this is the smallest addressable unit on this MCU core).Anyway, this is bad behavior for two reasons:
uint8_t
is defined to be 8 bit wide, not 16 bit. People might rely on that (for overflow behavior, e.g.). Defining auint8_t
to a 16bit wide type is no good idea.A solution would be to change this line to
typedef unsigned char uint_least8_t;
. That way it is indicated that the size is 8bit or more.Anyway, this should not be necessary, as at least current C2000 compilers already do exactly that.
In addition, it seems, that at least in CppUTest,
uint8_t
is not used at all.Thus, I recommend removing this line.
The text was updated successfully, but these errors were encountered: