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

Revert "Fix warnings" #3788

Closed
wants to merge 1 commit into from
Closed

Revert "Fix warnings" #3788

wants to merge 1 commit into from

Conversation

sauwming
Copy link
Member

Reverts #3778

To fix #3785

@sauwming sauwming added this to the release-2.15 milestone Nov 24, 2023
@sauwming sauwming closed this Nov 24, 2023
@nanangizz
Copy link
Member

Right, I think those originally are compile warnings treated as errors (e.g: due to -Werror=format). pj_size_t/pj_ssize_t can be long or not, so print format can be tricky. So I guess if we decide to use %ld/%lu, we need to ignore the format warnings when pj_size_t/pj_ssize_t is not using long?

@sauwming sauwming deleted the revert-3778-master branch November 24, 2023 02:48
@sauwming
Copy link
Member Author

Yes, it is tricky.
Initially I want to revert this commit and resort to using cast instead, i.e. "%u", (unsigned) ...., but then I realise that we have been using %lu everywhere else, so the problem is not with this commit.

It turns out that we need to use %zu instead for size_t:
https://cplusplus.com/reference/cstdio/printf/
https://www.tutorialspoint.com/what-is-the-correct-way-to-use-printf-to-print-a-size-t-in-c-cplusplus

@nanangizz
Copy link
Member

The %zu may have a drawback as it seems to be introduced in C99, while AFAIK the library has been trying to conform to C89.

See https://copyprogramming.com/howto/is-the-zu-specifier-required-for-printf

@sauwming
Copy link
Member Author

Ah okay, I guess we shouldn't use zu, it seems that it may cause issues with VS and mingw as well.

So I suppose the safest approach for us is to cast it to unsigned long, i.e. printf("%lu\n", (unsigned long)n)

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

Successfully merging this pull request may close these issues.

Cross compile doesn't work
2 participants