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

Improve compatibility with C++ consumers #10409

Merged
merged 1 commit into from
Jun 6, 2020
Merged

Conversation

ghost
Copy link

@ghost ghost commented Jun 5, 2020

Motivation and Context

FreeBSD has zfsd, which serves a similar purpose as zed. This program is written in C++.
C++ is a little picky about silly things like not using keywords for names and string const correctness.

Description

Mostly changed things in headers to rename parameters from class to clazz and private to priv.
One struct had a member named private which has been renamed to priv, and that required touching some .c files.
Corrected the constness of the name parameter to zpool_label_disk.

How Has This Been Tested?

Ran through ZTS on FreeBSD.

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.

@ghost ghost added the Status: Code Review Needed Ready for review and testing label Jun 5, 2020
@codecov
Copy link

codecov bot commented Jun 6, 2020

Codecov Report

Merging #10409 into master will decrease coverage by 0.08%.
The diff coverage is 76.92%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #10409      +/-   ##
==========================================
- Coverage   79.45%   79.37%   -0.09%     
==========================================
  Files         391      391              
  Lines      123625   123625              
==========================================
- Hits        98230    98131      -99     
- Misses      25395    25494      +99     
Flag Coverage Δ
#kernel 79.97% <75.00%> (+0.05%) ⬆️
#user 65.58% <69.23%> (+0.17%) ⬆️
Impacted Files Coverage Δ
include/sys/abd.h 100.00% <ø> (ø)
lib/libspl/include/assert.h 0.00% <0.00%> (ø)
module/os/linux/zfs/spa_stats.c 82.61% <71.42%> (ø)
include/sys/dsl_dataset.h 100.00% <100.00%> (ø)
include/sys/range_tree.h 87.50% <100.00%> (ø)
lib/libspl/include/umem.h 93.75% <100.00%> (ø)
lib/libzfs/os/linux/libzfs_pool_os.c 69.64% <100.00%> (ø)
cmd/zdb/zdb_il.c 30.86% <0.00%> (-24.08%) ⬇️
module/zcommon/zfs_fletcher.c 68.09% <0.00%> (-10.20%) ⬇️
module/zfs/vdev_indirect.c 74.66% <0.00%> (-10.00%) ⬇️
... and 64 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 99b281f...a1c5b1f. Read the comment docs.

@lundman
Copy link
Contributor

lundman commented Jun 6, 2020

I have this commit lined up for a PR:

openzfsonosx/openzfs@26fb082

there's probably a fair bit of overlap.

Copy link
Contributor

@behlendorf behlendorf left a comment

Choose a reason for hiding this comment

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

This looks good to me. Let's just verify that this PR covers everything that in @lundman's commit.

C++ is a little picky about not using keywords for names, or string
constness.

Signed-off-by: Ryan Moeller <ryan@iXsystems.com>
@ghost
Copy link
Author

ghost commented Jun 6, 2020

  • Fix a few more headers caught by @lundman

@lundman
Copy link
Contributor

lundman commented Jun 6, 2020

I replaced my commit with yours;

os/macos/libzfs_pool_os.c:215:1: error: conflicting types for 'zpool_label_disk'
zpool_label_disk(libzfs_handle_t *hdl, zpool_handle_t *zhp, char *name)

Updated lib/libzfs/os/macos/libzfs_pool_os.c - to add const.

All compiled - takes care of my needs, cheers.

@ghost
Copy link
Author

ghost commented Jun 6, 2020

That dang zpool_import_012 sure is stubborn :|

@behlendorf behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Jun 6, 2020
@behlendorf behlendorf merged commit 6026507 into openzfs:master Jun 6, 2020
@ghost ghost deleted the cxx branch June 6, 2020 20:50
BrainSlayer pushed a commit to BrainSlayer/zfs that referenced this pull request Jun 10, 2020
C++ is a little picky about not using keywords for names, or string
constness.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Jorgen Lundman <lundman@lundman.net>
Signed-off-by: Ryan Moeller <ryan@iXsystems.com>
Closes openzfs#10409
lundman referenced this pull request in openzfsonosx/openzfs Jun 12, 2020
C++ is a little picky about not using keywords for names, or string
constness.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Jorgen Lundman <lundman@lundman.net>
Signed-off-by: Ryan Moeller <ryan@iXsystems.com>
Closes #10409
jsai20 pushed a commit to jsai20/zfs that referenced this pull request Mar 30, 2021
C++ is a little picky about not using keywords for names, or string
constness.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Jorgen Lundman <lundman@lundman.net>
Signed-off-by: Ryan Moeller <ryan@iXsystems.com>
Closes openzfs#10409
sempervictus pushed a commit to sempervictus/zfs that referenced this pull request May 31, 2021
C++ is a little picky about not using keywords for names, or string
constness.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Jorgen Lundman <lundman@lundman.net>
Signed-off-by: Ryan Moeller <ryan@iXsystems.com>
Closes openzfs#10409
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Accepted Ready to integrate (reviewed, tested)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants