Skip to content

py/mpconfig.h: Define BITS_PER_BYTE only if not already defined. #5521

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jan 13, 2020
Merged

py/mpconfig.h: Define BITS_PER_BYTE only if not already defined. #5521

merged 1 commit into from
Jan 13, 2020

Conversation

Jongy
Copy link
Contributor

@Jongy Jongy commented Jan 12, 2020

It's a common macro that is possibly defined in headers of systems/SDKs
MicroPython is embedded into.

Required for #5482 (Linux defines it, so required to avoid double definition)

@Jongy
py/mpconfig.h: Define BITS_PER_BYTE only if not already defined.
1c849d6 
It's a common macro that is possibly defined in headers of systems/SDKs
MicroPython is embedded into.
@Jongy Jongy mentioned this pull request Jan 12, 2020
Open
8 tasks
@dpgeorge dpgeorge added the py-core Relates to py/ directory in source label Jan 13, 2020
@dpgeorge
Copy link
Member

The other option would be to rename uPy's constant to MP_BITS_PER_BYTE (everywhere). But this patch here is smaller and I don't see any harm using a name that's already defined. With a name like that it should be correctly defined if it's already defined!

@Jongy
Copy link
Contributor Author

Jongy commented Jan 13, 2020

I also think it's better to use well known names when possible. BITS_PER_BYTE is not standard C, but it's a de-facto standard in many projects.

@dpgeorge dpgeorge merged commit 1c849d6 into micropython:master Jan 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
py-core Relates to py/ directory in source
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Jongy @dpgeorge