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.

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

Prefix zfs internal endian checks with _ZFS #10621

Merged
merged 1 commit into from
Jul 28, 2020
Merged

Prefix zfs internal endian checks with _ZFS #10621

merged 1 commit into from
Jul 28, 2020

Conversation

mattmacy
Copy link
Contributor

FreeBSD defines _BIG_ENDIAN BIG_ENDIAN _LITTLE_ENDIAN
LITTLE_ENDIAN on every architecture. Trying to do
cross builds whilst hiding this from ZFS has proven
extremely cumbersome.

Signed-off-by: Matt Macy [email protected]

Motivation and Context

Description

How Has This Been Tested?

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Performance enhancement (non-breaking change which improves efficiency)
  • Code cleanup (non-breaking change which makes code smaller or more readable)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation (a change to man pages or other documentation)

Checklist:

  • My code follows the ZFS on Linux code style requirements.
  • I have updated the documentation accordingly.
  • I have read the contributing document.
  • I have added tests to cover my changes.
  • I have run the ZFS Test Suite with this change applied.
  • All commit messages are properly formatted and contain Signed-off-by.

@behlendorf behlendorf added the Status: Code Review Needed Ready for review and testing label Jul 24, 2020
@mattmacy mattmacy force-pushed the projects/endian_rename branch 2 times, most recently from 7299125 to 7a36fe0 Compare July 24, 2020 03:13
@mattmacy mattmacy mentioned this pull request Jul 24, 2020
Merged
12 tasks
@codecov
Copy link

codecov bot commented Jul 24, 2020 •
edited
Loading

Codecov Report

Merging #10621 into master will decrease coverage by 0.06%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #10621      +/-   ##
==========================================
- Coverage   79.66%   79.59%   -0.07%     
==========================================
  Files         394      394              
  Lines      124629   124611      -18     
==========================================
- Hits        99280    99187      -93     
- Misses      25349    25424      +75
Flag Coverage Δ
#kernel 80.17% <ø> (-0.24%) ⬇️
#user 65.86% <ø> (+0.69%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
include/os/linux/spl/sys/byteorder.h 100.00% <ø> (ø)
lib/libspl/include/os/linux/sys/byteorder.h 100.00% <ø> (ø)
module/icp/algs/aes/aes_impl_generic.c 70.37% <ø> (ø)
module/icp/algs/modes/ccm.c 84.36% <ø> (ø)
module/icp/algs/sha1/sha1.c 0.00% <ø> (ø)
module/icp/algs/sha2/sha2.c 97.40% <ø> (ø)
module/nvpair/nvpair.c 83.26% <ø> (ø)
module/unicode/uconv.c 0.00% <ø> (ø)
module/zfs/lz4.c 98.58% <ø> (ø)
module/os/linux/spl/spl-zlib.c 55.35% <0.00%> (-28.58%) ⬇️
... and 58 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e64cc49...8dfe356. Read the comment docs.

@mattmacy mattmacy force-pushed the projects/endian_rename branch from 7a36fe0 to bd5f7b4 Compare July 24, 2020 15:36
ghost
ghost reviewed Jul 24, 2020
View reviewed changes
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

I also see some leftovers of the old names in various comments and in

  • cmd/zdb/zdb.c
  • lib/libzpool/util.c
  • module/icp/algs/sha1/sha1.c (this one maybe makes sense?)

lib/libspl/include/os/freebsd/sys/byteorder.h Outdated Show resolved Hide resolved
lib/libspl/include/os/freebsd/sys/byteorder.h Outdated Show resolved Hide resolved
lib/libspl/include/os/freebsd/sys/byteorder.h Outdated Show resolved Hide resolved
@mattmacy mattmacy force-pushed the projects/endian_rename branch 7 times, most recently from 14ab9c4 to 3ecdb78 Compare July 25, 2020 01:12
@ghost ghost force-pushed the projects/endian_rename branch from 3ecdb78 to d4ab403 Compare July 25, 2020 21:38
@ghost
Copy link

ghost commented Jul 25, 2020 •
edited by ghost
Loading

  • Rebased
  • Define _ZFS_(BIG|LITTLE)_ENDIAN if _(BIG|LITTLE)_ENDIAN is already defined (should fix powerpc)

@ghost ghost force-pushed the projects/endian_rename branch from d4ab403 to c474a21 Compare July 25, 2020 22:18
@ghost
Copy link

ghost commented Jul 25, 2020

  • s/define/defined

@ghost ghost force-pushed the projects/endian_rename branch from c474a21 to d465d4f Compare July 25, 2020 23:20
@ghost
Copy link

ghost commented Jul 25, 2020

  • Instead of checking _(BIG|LITTLE)_ENDIAN which is exactly what we're trying to avoid, check __(BIG|LITTLE)_ENDIAN in a better position

@ghost ghost force-pushed the projects/endian_rename branch from d465d4f to ab92107 Compare July 26, 2020 00:32
@ghost
Copy link

ghost commented Jul 26, 2020

  • Same for the libspl version of byteorder.h

@ghost ghost force-pushed the projects/endian_rename branch 4 times, most recently from b833ee5 to 0db1d21 Compare July 26, 2020 18:46
@mattmacy mattmacy force-pushed the projects/endian_rename branch from 0db1d21 to 49ef922 Compare July 27, 2020 20:05
@mattmacy
Prefix zfs internal endian checks with _ZFS
8dfe356 
FreeBSD defines _BIG_ENDIAN BIG_ENDIAN _LITTLE_ENDIAN
LITTLE_ENDIAN on every architecture. Trying to do
cross builds whilst hiding this from ZFS has proven
extremely cumbersome.

Signed-off-by: Matt Macy <[email protected]>
@mattmacy mattmacy force-pushed the projects/endian_rename branch from 49ef922 to 1a69251 Compare July 27, 2020 20:46
@behlendorf behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Jul 27, 2020
@mattmacy mattmacy force-pushed the projects/endian_rename branch from 1a69251 to 8dfe356 Compare July 28, 2020 03:28
@behlendorf behlendorf merged commit 5678d3f into openzfs:master Jul 28, 2020
@mattmacy mattmacy deleted the projects/endian_rename branch August 6, 2020 22:11
jsai20 pushed a commit to jsai20/zfs that referenced this pull request Mar 30, 2021
@mattmacy @jsai20
Prefix zfs internal endian checks with _ZFS
18bdcff 
FreeBSD defines _BIG_ENDIAN BIG_ENDIAN _LITTLE_ENDIAN
LITTLE_ENDIAN on every architecture. Trying to do
cross builds whilst hiding this from ZFS has proven
extremely cumbersome.

Reviewed-by: Ryan Moeller <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Matt Macy <[email protected]>
Closes openzfs#10621
sempervictus pushed a commit to sempervictus/zfs that referenced this pull request May 31, 2021
@mattmacy
Prefix zfs internal endian checks with _ZFS
81baf3b 
FreeBSD defines _BIG_ENDIAN BIG_ENDIAN _LITTLE_ENDIAN
LITTLE_ENDIAN on every architecture. Trying to do
cross builds whilst hiding this from ZFS has proven
extremely cumbersome.

Reviewed-by: Ryan Moeller <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Matt Macy <[email protected]>
Closes openzfs#10621
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@behlendorf behlendorf behlendorf approved these changes

Assignees
No one assigned
Labels
Status: Accepted Ready to integrate (reviewed, tested)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@mattmacy @behlendorf