Skip to content
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

Open
fgrie opened this issue May 10, 2024 · 3 comments
Open

Change or remove include/Platforms/c2000/stdint.h #1792

fgrie opened this issue May 10, 2024 · 3 comments

Comments

@fgrie
Copy link

fgrie commented May 10, 2024

The named file defines uint8_t for C2000 MCUs using the following line:

typedef unsigned char uint8_t; /* This will still compile to 16 bit */

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:

  1. A uint8_t is defined to be 8 bit wide, not 16 bit. People might rely on that (for overflow behavior, e.g.). Defining a uint8_t to a 16bit wide type is no good idea.
  2. It usually is considered bad to re-define standard types (even if they are not available on this platform)

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.

@thetic
Copy link
Contributor

thetic commented May 10, 2024

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.

@fgrie
Copy link
Author

fgrie commented May 11, 2024

I can try, but I'm not sure where to start.

  • The toolchain used for the tests is far outdated (even if the exact version is nowhere stated).
  • Update to a new toolchain or stay with the old one?
  • I'm not even sure if the old one is still available, anyway, the current version should be able to import old projects (creating a new project would also not be rocket science)
  • If built using a new IDE version, also the new compiler will be used, which is likely to be incompatible to the existing code
  • Testing: TI moved away from their C2000 simulator, and recommends their Launchpad evaluation boards. Good for eval board sales, bad for cloud-based CI

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.)?

@basvodde
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants