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

Destroy legacy string functions. Clean out vtoc.h #12996

Closed
wants to merge 19 commits into from

Conversation

nabijaczleweli
Copy link
Contributor

@nabijaczleweli nabijaczleweli commented Jan 22, 2022

Motivation and Context

All my homies hate the legacy string functions. Also, the freebsd strings.h stub said "do not remove", which is irresistible.

Description

Draft because on top of #12901, otherwise g2g.

nabijaczleweli@tarta:~/store/code/zfs$ git grep -E 'b(cmp|copy|zero)'
config/Rules.am:AM_CPPFLAGS += -D"bcopy(...)=bcopy(__VA_ARGS__) __attribute__((deprecated(\"bcopy(3) is deprecated. Use memcpy(3)/memmove(3) instead!\")))"
config/Rules.am:AM_CPPFLAGS += -D"bcmp(...)=bcmp(__VA_ARGS__) __attribute__((deprecated(\"bcmp(3) is deprecated. Use memcmp(3) instead!\")))"
config/Rules.am:AM_CPPFLAGS += -D"bzero(...)=bzero(__VA_ARGS__) __attribute__((deprecated(\"bzero(3) is deprecated. Use memset(3) instead!\")))"
include/sys/arc.h:arc_read_done_func_t arc_bcopy_func;
module/zfs/arc.c:arc_bcopy_func(zio_t *zio, const zbookmark_phys_t *zb, const blkptr_t *bp,
scripts/cstyle.pl:      if (/!\s*(strcmp|strncmp|bcmp)\s*\(/) {
nabijaczleweli@tarta:~/store/code/zfs$ git grep -E 'b(cmp|copy|zero)' | wc
      6      41     649

down from, uh,

nabijaczleweli@tarta:~/store/code/zfs$ git grep -E 'b(cmp|copy|zero)' origin/master | wc
    898    4089   76557
nabijaczleweli@tarta:~/store/code/zfs$ git grep -E 'b(cmp|copy|zero)' origin/master | cut -d: -f2 | uniq | wc
    135     135    3746

Also, I went to town on vtoc.h. Maybe a little too to town? But I don't think any mercy should be shown.

Also also I made zpool wait -t (and all other users of getsubopt(3)) accept just the word[,...]s instead of word[=whatever][,...] (for a delightfully const moment).

How Has This Been Tested?

Builds.

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) – I mean, probably, given that most memcpys are memcpys instead of memmoves. That being said, this is really more notional I think.
  • Code cleanup (non-breaking change which makes code smaller or more readable)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
  • Documentation (a change to man pages or other documentation)

Checklist:

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

@nabijaczleweli nabijaczleweli changed the title Based copy Destroy legacy string functions. Clean out vtoc.h Jan 22, 2022
@nabijaczleweli nabijaczleweli force-pushed the based-copy branch 2 times, most recently from efa0331 to 8f76c0f Compare January 22, 2022 13:01
@sempervictus
Copy link
Contributor

sempervictus commented Jan 22, 2022

Prolific, thank you :). I wonder if there are any out of tree consumers for this stuff like GRUB or what-not which may expect the symbols to be around for something. By no means suggesting to let dead code lie, just wondering out loud what weird uses the larger ecosystem may have for some of these legacy paths.
What's the appropriate translation for достопримечательность in this context anyway (given that it's bit expands to "it is" vs "belonging to it")? I'm guessing intended to convey the presence of specific semantics at those sites, though in that colloquial/inferential meaning i'm missing the nuanced "why" piece (that being why it is достойено к примечанию).

@nabijaczleweli nabijaczleweli force-pushed the based-copy branch 2 times, most recently from 5295864 to e66c71c Compare January 22, 2022 15:37
@nabijaczleweli
Copy link
Contributor Author

VTOC is (I'm guessing from the headers here) an AT&T UNIX disk labelling system. Crookedly reproduced in Solaris. Not reproduced outside. The SYSCALL32 stuff, on the other hand, is for 32-bit-userland-on-64-bit-kernels (think i686 on and amd64 kernel, would x32 produce another, third, set of definitions? doesn't bear thinking about). Nothing uses this. I guess the error codes and (partial?) MBR(?) partition code lists may be used outside, but. Well. It's libefi. Decidedly the worst and most annoying bit of libzfs, and I'd say openzfs as it stands today (and it's for writes, which neither bootloaders nor GRUB do (well, GRUB does have a single persistent block, but, you know, it's different)). I yearn for the day when I remove it entirely. Hopefully.

Also, GRUB? You do know that GRUB doesn't (cannot) link to Solaris code because the GPL is too proprietary to be compatible with the CDDL, right? This is the primary reason it's absolute hot shit and not worth the effort to do boot pools (especially on EFI).

As for достопримечательности, it's "places worth visiting" in a touristy sense, I guess? Not necessarily sights to be seen, since those are natural sites and достопримечательности aren't necessarily. Dunno, its like a vibe, too. In lieu of a translation I recommend being slavic and speaking russian from an early age (though, indeed, that may be a little too aspirational for the casual reader).

@gmelikov
Copy link
Member

Offtopic, an interesting and close sense of meaning is attraction in a way of place/thing.

@nabijaczleweli nabijaczleweli force-pushed the based-copy branch 11 times, most recently from 18a4486 to b254c8d Compare January 23, 2022 01:13
@behlendorf behlendorf added the Status: Code Review Needed Ready for review and testing label Jan 24, 2022
@nabijaczleweli nabijaczleweli force-pushed the based-copy branch 2 times, most recently from 0923154 to aaddab7 Compare February 2, 2022 15:33
@nabijaczleweli nabijaczleweli force-pushed the based-copy branch 3 times, most recently from b4c3b21 to 87f6255 Compare February 16, 2022 00:45
@nabijaczleweli nabijaczleweli marked this pull request as ready for review February 16, 2022 00:45
@nabijaczleweli
Copy link
Contributor Author

Rebased, g2g

andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Sep 23, 2022
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#12996
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Sep 23, 2022
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#12996
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Sep 23, 2022
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#12996
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Sep 23, 2022
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#12996
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Sep 23, 2022
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#12996
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Sep 23, 2022
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#12996
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Sep 23, 2022
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#12996
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Sep 23, 2022
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#12996
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Sep 23, 2022
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#12996
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Sep 23, 2022
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#12996
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Sep 23, 2022
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#12996
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Sep 23, 2022
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#12996
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Sep 23, 2022
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#12996
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Sep 23, 2022
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#12996
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Oct 1, 2022
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#12996
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Oct 1, 2022
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#12996
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Oct 1, 2022
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#12996
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Oct 1, 2022
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#12996
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Oct 1, 2022
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#12996
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Oct 1, 2022
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#12996
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Oct 1, 2022
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#12996
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Oct 1, 2022
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#12996
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Oct 1, 2022
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#12996
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Oct 1, 2022
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#12996
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Oct 2, 2022
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#12996
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Oct 6, 2022
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#12996
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Oct 6, 2022
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#12996
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Oct 6, 2022
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#12996
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Oct 9, 2022
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#12996
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.

4 participants