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

Fix regression on bigendian #58

Merged
merged 1 commit into from
Jun 5, 2023

Conversation

remicollet
Copy link
Contributor

  • rewrite IP2Location_read32_row
  • more robust detection of endianness

Fedora scratch build using this fix:
https://koji.fedoraproject.org/koji/taskinfo?taskID=101823859

Fix #57

@remicollet
Copy link
Contributor Author

Perhaps WORDS_BIGENDIAN (from configure) can be used, but I don't know if AC_C_BIGENDIAN is reliable.

For ex, PHP use its own function, see https://github.com/php/php-src/blob/php-8.2.6/build/php.m4#L1691

Copy link
Contributor

@pbiering pbiering left a comment

Choose a reason for hiding this comment

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

Looks good

@sharkcz
Copy link

sharkcz commented Jun 5, 2023

Or the gcc (and other compilers) define __BYTE_ORDER__ and comparison to ____ORDER_BIG_ENDIAN__ can be used.

The AC_C_BIGENDIAN check works well too.

@pbiering
Copy link
Contributor

pbiering commented Jun 5, 2023

@remicollet : would you adjust your PR according to @sharkcz comments? Then I will wait creating RPMs.

@remicollet
Copy link
Contributor Author

@sharkcz @pbiering running test build using WORDS_BIGENDIAN

- rewrite IP2Location_read32_row
- more robust detection of endianness using WORDS_BIGENDIAN from AC_C_BIGENDIAN
@remicollet
Copy link
Contributor Author

Scratch build seems OK https://koji.fedoraproject.org/koji/taskinfo?taskID=101826163

PR updated

@remicollet remicollet requested a review from pbiering June 5, 2023 06:49
@remicollet remicollet changed the title Fix for bigendian Fix regression on bigendian Jun 5, 2023
Copy link
Contributor

@pbiering pbiering left a comment

Choose a reason for hiding this comment

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

Looks even better as the former one!

@ip2location ip2location merged commit 480c758 into chrislim2888:master Jun 5, 2023
@remicollet remicollet deleted the issue-bigend branch June 6, 2023 05:32
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.

Broken on bigendian (regression)
4 participants