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

Add header guards and fix some compiler warnings #29

Merged
merged 4 commits into from
Jul 19, 2023
Merged

Conversation

mlund
Copy link
Collaborator

@mlund mlund commented Jul 18, 2023

  • Add missing header guards
  • Unify header define names
  • Fix a few mainly integer size related warnings

@mlund mlund added the enhancement New feature or request label Jul 18, 2023
include/fcio.h Show resolved Hide resolved
include/debug.h Outdated Show resolved Hide resolved
@mlund mlund merged commit bac4ce3 into MEGA65:master Jul 19, 2023
@@ -837,4 +837,4 @@ void flushkeybuf(void);
unsigned char cinput(
unsigned char* buffer, unsigned char buflen, unsigned char flags);

#endif // M65LIBC_CONIO_H
#endif /* __MEGA65_CONIO_H */
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On this one /* .. */ is used, but the rest in the PR uses //. It would hurt if it was consistent, using // is C99 and will not work with C89/C94. C99 is close to 25 years old, but I do not know if we expect issues with old compilers?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh ok, I think we can resolve it in a separate issue, either now or when a new compiler becomes relevant. I'm unsure of what standards are out there.

@@ -180,6 +180,7 @@ void mega65_sdcard_unmap_sector_buffer(void)

unsigned short timeout;

// @todo Return -1 corresponds to 255. Is this what we want?
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume all bits set is an error value. This corresponds a bit to EOF which is used by some C library functions for error result.
Typically such cases use int in as the return type. This may be less ideal on an 8-bit CPU when given the choice of return type.
Personally I think it is ugly to return -1 when the return type is unsigned. One can use ~0 or ~0u, or perhaps better a macro to represent it.
Another way is to use int8_t as the return type which would signal that one can test for error by testing if the result is less than 0. In such case a negative value does not look inappropriate, which it kind of does now.

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

Successfully merging this pull request may close these issues.

2 participants