From b29bf2f84ea838b1b7dad4e80858b637395930ae Mon Sep 17 00:00:00 2001 From: Kyle Evans Date: Wed, 25 Jul 2018 15:14:35 +0000 Subject: [PATCH] libbe(3)/be(8): Drop WARNS overrides, fix all fallout Based on the idea that we shouldn't have all-new library and utility going into base that need WARNS=1... - Decent amount of constification - Lots of parentheses - Minor other nits --- cddl/lib/libbe/Makefile | 2 -- lib/libbe/be.c | 64 ++++++++++++++++++++--------------------- lib/libbe/be.h | 13 +++++---- lib/libbe/be_access.c | 11 ++++--- lib/libbe/be_impl.h | 6 ++-- lib/libbe/be_info.c | 2 +- lib/libbe/libbe.3 | 10 +++---- sbin/bectl/Makefile | 2 -- sbin/bectl/bectl.c | 14 ++++----- 9 files changed, 57 insertions(+), 67 deletions(-) diff --git a/cddl/lib/libbe/Makefile b/cddl/lib/libbe/Makefile index c15e7c9ebcf6..c71d59eaa334 100644 --- a/cddl/lib/libbe/Makefile +++ b/cddl/lib/libbe/Makefile @@ -11,8 +11,6 @@ SRCS= be.c be_access.c be_error.c be_info.c INCS= be.h MAN= libbe.3 -WARNS?= 1 - LIBADD+= zfs LIBADD+= nvpair diff --git a/lib/libbe/be.c b/lib/libbe/be.c index 934cdaf21f9f..48bb36c2beea 100644 --- a/lib/libbe/be.c +++ b/lib/libbe/be.c @@ -139,7 +139,7 @@ libbe_init(void) zfs_iter_filesystems(rootds, be_locate_rootfs, lbh); zfs_close(rootds); rootds = NULL; - if (lbh->rootfs == NULL) + if (*lbh->rootfs == '\0') goto err; return (lbh); @@ -212,7 +212,7 @@ be_destroy(libbe_handle_t *lbh, char *name, int options) return (set_error(lbh, BE_ERR_ZFSOPEN)); /* Check if mounted, unmount if force is specified */ - if (mounted = zfs_is_mounted(fs, NULL)) { + if ((mounted = zfs_is_mounted(fs, NULL)) != 0) { if (force) zfs_unmount(fs, NULL, 0); else @@ -230,7 +230,7 @@ be_destroy(libbe_handle_t *lbh, char *name, int options) int -be_snapshot(libbe_handle_t *lbh, char *source, char *snap_name, +be_snapshot(libbe_handle_t *lbh, const char *source, const char *snap_name, bool recursive, char *result) { char buf[BE_MAXPATHLEN]; @@ -257,7 +257,7 @@ be_snapshot(libbe_handle_t *lbh, char *source, char *snap_name, strcpy(result, strrchr(buf, '/') + 1); } - if (err = zfs_snapshot(lbh->lzh, buf, recursive, NULL) != 0) { + if ((err = zfs_snapshot(lbh->lzh, buf, recursive, NULL)) != 0) { switch (err) { case EZFS_INVALIDNAME: return (set_error(lbh, BE_ERR_INVALIDNAME)); @@ -280,7 +280,7 @@ be_create(libbe_handle_t *lbh, char *name) { int err; - err = be_create_from_existing(lbh, name, (char *)be_active_path(lbh)); + err = be_create_from_existing(lbh, name, be_active_path(lbh)); return (set_error(lbh, err)); } @@ -327,7 +327,6 @@ be_deep_clone(zfs_handle_t *ds, void *data) int err; char be_path[BE_MAXPATHLEN]; char snap_path[BE_MAXPATHLEN]; - char mp[BE_MAXPATHLEN]; const char *dspath; char *dsname; zfs_handle_t *snap_hdl; @@ -364,7 +363,7 @@ be_deep_clone(zfs_handle_t *ds, void *data) ZFS_TYPE_FILESYSTEM) == ZPROP_INVAL) return (-1); - if (err = zfs_clone(snap_hdl, be_path, props)) { + if ((err = zfs_clone(snap_hdl, be_path, props)) != 0) { switch (err) { case EZFS_SUCCESS: err = BE_ERR_SUCCESS; @@ -392,23 +391,25 @@ be_deep_clone(zfs_handle_t *ds, void *data) * Create the boot environment from pre-existing snapshot */ int -be_create_from_existing_snap(libbe_handle_t *lbh, char *name, char *snap) +be_create_from_existing_snap(libbe_handle_t *lbh, const char *name, + const char *snap) { int err; char be_path[BE_MAXPATHLEN]; char snap_path[BE_MAXPATHLEN]; - char *parentname, *bename, *snapname; + const char *bename; + char *parentname, *snapname; zfs_handle_t *parent_hdl; struct libbe_deep_clone sdc; - if (err = be_validate_name(lbh, name)) + if ((err = be_validate_name(lbh, name)) != 0) return (set_error(lbh, err)); - if (err = be_root_concat(lbh, snap, snap_path)) + if ((err = be_root_concat(lbh, snap, snap_path)) != 0) return (set_error(lbh, err)); - if (err = be_validate_snap(lbh, snap_path)) + if ((err = be_validate_snap(lbh, snap_path)) != 0) return (set_error(lbh, err)); - if (err = be_root_concat(lbh, name, be_path)) + if ((err = be_root_concat(lbh, name, be_path)) != 0) return (set_error(lbh, err)); if ((bename = strrchr(name, '/')) == NULL) @@ -444,7 +445,7 @@ be_create_from_existing_snap(libbe_handle_t *lbh, char *name, char *snap) * Create a boot environment from an existing boot environment */ int -be_create_from_existing(libbe_handle_t *lbh, char *name, char *old) +be_create_from_existing(libbe_handle_t *lbh, const char *name, const char *old) { int err; char buf[BE_MAXPATHLEN]; @@ -464,12 +465,11 @@ be_create_from_existing(libbe_handle_t *lbh, char *name, char *old) * failure. Does not set the internal library error state. */ int -be_validate_snap(libbe_handle_t *lbh, char *snap_name) +be_validate_snap(libbe_handle_t *lbh, const char *snap_name) { zfs_handle_t *zfs_hdl; char buf[BE_MAXPATHLEN]; char *delim_pos; - char *mountpoint; int err = BE_ERR_SUCCESS; if (strlen(snap_name) >= BE_MAXPATHLEN) @@ -490,8 +490,8 @@ be_validate_snap(libbe_handle_t *lbh, char *snap_name) zfs_open(lbh->lzh, buf, ZFS_TYPE_DATASET)) == NULL) return (BE_ERR_NOORIGIN); - if (err = zfs_prop_get(zfs_hdl, ZFS_PROP_MOUNTPOINT, buf, BE_MAXPATHLEN, - NULL, NULL, 0, 1)) + if ((err = zfs_prop_get(zfs_hdl, ZFS_PROP_MOUNTPOINT, buf, BE_MAXPATHLEN, + NULL, NULL, 0, 1)) != 0) err = BE_ERR_INVORIGIN; if ((err != 0) && (strncmp(buf, "/", BE_MAXPATHLEN) != 0)) @@ -512,7 +512,7 @@ be_validate_snap(libbe_handle_t *lbh, char *snap_name) * zfs_be_root. Does not set internal library error state. */ int -be_root_concat(libbe_handle_t *lbh, char *name, char *result) +be_root_concat(libbe_handle_t *lbh, const char *name, char *result) { size_t name_len, root_len; @@ -545,7 +545,7 @@ be_root_concat(libbe_handle_t *lbh, char *name, char *result) * Does not set internal library error state. */ int -be_validate_name(libbe_handle_t *lbh, char *name) +be_validate_name(libbe_handle_t *lbh __unused, const char *name) { for (int i = 0; *name; i++) { char c = *(name++); @@ -569,12 +569,12 @@ be_rename(libbe_handle_t *lbh, char *old, char *new) zfs_handle_t *zfs_hdl; int err; - if (err = be_root_concat(lbh, old, full_old)) + if ((err = be_root_concat(lbh, old, full_old)) != 0) return (set_error(lbh, err)); - if (err = be_root_concat(lbh, new, full_new)) + if ((err = be_root_concat(lbh, new, full_new)) != 0) return (set_error(lbh, err)); - if (be_validate_name(lbh, new)) + if (be_validate_name(lbh, new) != 0) return (BE_ERR_UNKNOWN); /* XXX TODO set and return correct error */ @@ -621,7 +621,7 @@ be_export(libbe_handle_t *lbh, char *bootenv, int fd) zfs_handle_t *zfs; int err; - if (err = be_snapshot(lbh, bootenv, NULL, true, snap_name)) + if ((err = be_snapshot(lbh, bootenv, NULL, true, snap_name)) != 0) /* XXX TODO error handle */ return (-1); @@ -648,7 +648,7 @@ be_import(libbe_handle_t *lbh, char *bootenv, int fd) * XXX TODO: this is a very likely name for someone to already have * used... we should avoid it. */ - if (err = be_root_concat(lbh, "be_import_temp", buf)) + if ((err = be_root_concat(lbh, "be_import_temp", buf)) != 0) /* XXX TODO error handle */ return (-1); @@ -658,7 +658,7 @@ be_import(libbe_handle_t *lbh, char *bootenv, int fd) "@%F-%T", localtime(&rawtime)); /* lzc_receive(SNAPNAME, PROPS, ORIGIN, FORCE, fd)) { */ - if (err = lzc_receive(buf, NULL, NULL, false, fd)) { + if ((err = lzc_receive(buf, NULL, NULL, false, fd)) != 0) { /* TODO: go through libzfs_core's recv_impl and find returned * errors and set appropriate BE_ERR * edit: errors are not in libzfs_core, my assumption is @@ -737,8 +737,8 @@ be_add_child(libbe_handle_t *lbh, char *child_path, bool cp_if_exists) nvlist_add_string(props, "mountpoint", child_path); /* Create */ - if (err = - zfs_create(lbh->lzh, active, ZFS_TYPE_DATASET, props)) + if ((err = + zfs_create(lbh->lzh, active, ZFS_TYPE_DATASET, props)) != 0) /* XXX TODO handle error */ return (-1); nvlist_free(props); @@ -749,7 +749,7 @@ be_add_child(libbe_handle_t *lbh, char *child_path, bool cp_if_exists) return (-1); /* Set props */ - if (err = zfs_prop_set(zfs, "canmount", "noauto")) + if ((err = zfs_prop_set(zfs, "canmount", "noauto")) != 0) /* TODO handle error */ return (-1); } else if (cp_if_exists) { @@ -767,7 +767,7 @@ be_add_child(libbe_handle_t *lbh, char *child_path, bool cp_if_exists) snprintf(buf, BE_MAXPATHLEN, "%s@%ld", child_path, snap_name); - if (err = zfs_snapshot(lbh->lzh, buf, false, NULL)) + if ((err = zfs_snapshot(lbh->lzh, buf, false, NULL)) != 0) /* XXX TODO correct error */ return (-1); @@ -777,7 +777,7 @@ be_add_child(libbe_handle_t *lbh, char *child_path, bool cp_if_exists) /* XXX TODO correct error */ return (-1); - if (err = zfs_clone(zfs, active, NULL)) + if ((err = zfs_clone(zfs, active, NULL)) != 0) /* XXX TODO correct error */ return (-1); @@ -797,8 +797,6 @@ be_activate(libbe_handle_t *lbh, char *bootenv, bool temporary) char buf[BE_MAXPATHLEN]; uint64_t pool_guid; uint64_t vdev_guid; - int zfs_fd; - int len; int err; be_root_concat(lbh, bootenv, be_path); diff --git a/lib/libbe/be.h b/lib/libbe/be.h index 2727c1cdfeee..1b3218c206a4 100644 --- a/lib/libbe/be.h +++ b/lib/libbe/be.h @@ -67,14 +67,15 @@ const char *be_nextboot_path(libbe_handle_t *); const char *be_root_path(libbe_handle_t *); int be_get_bootenv_props(libbe_handle_t *, nvlist_t *); +void be_prop_list_free(nvlist_t *be_list); int be_activate(libbe_handle_t *, char *, bool); /* Bootenv creation functions */ int be_create(libbe_handle_t *, char *); -int be_create_from_existing(libbe_handle_t *, char *, char *); -int be_create_from_existing_snap(libbe_handle_t *, char *, char *); -int be_snapshot(libbe_handle_t *, char *, char *, bool, char *); +int be_create_from_existing(libbe_handle_t *, const char *, const char *); +int be_create_from_existing_snap(libbe_handle_t *, const char *, const char *); +int be_snapshot(libbe_handle_t *, const char *, const char *, bool, char *); /* Bootenv manipulation functions */ int be_rename(libbe_handle_t *, char *, char *); @@ -103,9 +104,9 @@ const char *libbe_error_description(libbe_handle_t *); void libbe_print_on_error(libbe_handle_t *, bool); /* Utility Functions */ -int be_root_concat(libbe_handle_t *, char *, char *); -int be_validate_name(libbe_handle_t *, char *); -int be_validate_snap(libbe_handle_t *, char *); +int be_root_concat(libbe_handle_t *, const char *, char *); +int be_validate_name(libbe_handle_t * __unused, const char *); +int be_validate_snap(libbe_handle_t *, const char *); bool be_exists(libbe_handle_t *, char *); int be_export(libbe_handle_t *, char *, int fd); diff --git a/lib/libbe/be_access.c b/lib/libbe/be_access.c index ebd196f99dec..58176a948222 100644 --- a/lib/libbe/be_access.c +++ b/lib/libbe/be_access.c @@ -38,12 +38,11 @@ be_mount(libbe_handle_t *lbh, char *bootenv, char *mountpoint, int flags, { char be[BE_MAXPATHLEN]; char mnt_temp[BE_MAXPATHLEN]; - zfs_handle_t *zfs_hdl; char *path; int mntflags; int err; - if (err = be_root_concat(lbh, bootenv, be)) + if ((err = be_root_concat(lbh, bootenv, be)) != 0) return (set_error(lbh, err)); if (!be_exists(lbh, bootenv)) @@ -63,8 +62,8 @@ be_mount(libbe_handle_t *lbh, char *bootenv, char *mountpoint, int flags, } char opt = '\0'; - if (err = zmount(be, (mountpoint == NULL) ? mnt_temp : mountpoint, - mntflags, MNTTYPE_ZFS, NULL, 0, &opt, 1)) + if ((err = zmount(be, (mountpoint == NULL) ? mnt_temp : mountpoint, + mntflags, __DECONST(char *, MNTTYPE_ZFS), NULL, 0, &opt, 1)) != 0) /* * XXX TODO: zmount returns the nmount error, look into what * kind of errors we can report from that @@ -90,7 +89,7 @@ be_unmount(libbe_handle_t *lbh, char *bootenv, int flags) int mntsize; char *mntpath; - if (err = be_root_concat(lbh, bootenv, be)) + if ((err = be_root_concat(lbh, bootenv, be)) != 0) return (set_error(lbh, err)); if ((mntsize = getmntinfo(&mntbuf, MNT_NOWAIT)) == 0) @@ -114,7 +113,7 @@ be_unmount(libbe_handle_t *lbh, char *bootenv, int flags) mntflags = (flags & BE_MNT_FORCE) ? MNT_FORCE : 0; - if (err = unmount(mntpath, mntflags)) + if ((err = unmount(mntpath, mntflags)) != 0) /* XXX TODO correct error */ return (set_error(lbh, BE_ERR_NOMOUNT)); diff --git a/lib/libbe/be_impl.h b/lib/libbe/be_impl.h index 923b34d50a8a..73f5c22e386e 100644 --- a/lib/libbe/be_impl.h +++ b/lib/libbe/be_impl.h @@ -46,9 +46,9 @@ struct libbe_handle { struct libbe_deep_clone { libbe_handle_t *lbh; - char *bename; - char *snapname; - char *be_root; + const char *bename; + const char *snapname; + const char *be_root; }; struct libbe_dccb { diff --git a/lib/libbe/be_info.c b/lib/libbe/be_info.c index 6a7e445a5d56..644a93510cdf 100644 --- a/lib/libbe/be_info.c +++ b/lib/libbe/be_info.c @@ -119,7 +119,7 @@ prop_list_builder_cb(zfs_handle_t *zfs_hdl, void *data_p) libbe_handle_t *lbh; nvlist_t *props; const char *dataset, *name; - boolean_t mounted, active, nextboot; + boolean_t mounted; /* * XXX TODO: diff --git a/lib/libbe/libbe.3 b/lib/libbe/libbe.3 index 191840b60338..2ddb2b0a0c58 100644 --- a/lib/libbe/libbe.3 +++ b/lib/libbe/libbe.3 @@ -30,7 +30,7 @@ .\" .\" $FreeBSD$ .\" -.Dd August 28, 2017 +.Dd July 25, 2018 .Dt LIBBE 3 .Os .Sh NAME @@ -76,7 +76,7 @@ of state to be retained, such as errors from previous operations. .Fn be_create "libbe_handle_t *, char *" ; .Pp .Ft int -.Fn be_create_from_existing "libbe_handle_t *, char *, char *" ; +.Fn be_create_from_existing "libbe_handle_t *, const char *, const char *" ; .Pp .Ft int .Fn be_rename "libbe_handle_t *, char *, char *" ; @@ -109,13 +109,13 @@ of state to be retained, such as errors from previous operations. .Fn libbe_print_on_error "libbe_handle_t *, bool" ; .Pp .Ft int -.Fn be_root_concat "libbe_handle_t *, char *, char *" ; +.Fn be_root_concat "libbe_handle_t *, const char *, char *" ; .Pp .Ft int -.Fn be_validate_name "libbe_handle_t *, char *" ; +.Fn be_validate_name "libbe_handle_t *, const char *" ; .Pp .Ft int -.Fn be_validate_snap "libbe_handle_t *, char *" ; +.Fn be_validate_snap "libbe_handle_t *, const char *" ; .Pp .Ft bool .Fn be_exists "libbe_handle_t *, char *" ; diff --git a/sbin/bectl/Makefile b/sbin/bectl/Makefile index 6f635a71d9bf..0a03f67d964e 100644 --- a/sbin/bectl/Makefile +++ b/sbin/bectl/Makefile @@ -1,7 +1,6 @@ # $FreeBSD$ PROG= bectl -WARNS?= 1 MAN= bectl.8 LIBADD+= be @@ -9,7 +8,6 @@ LIBADD+= nvpair CFLAGS+= -I${SRCTOP}/cddl/contrib/opensolaris/lib/libzfs/common CFLAGS+= -I${SRCTOP}/sys/cddl/compat/opensolaris -CFLAGS+= -I${SRCTOP}/sys/cddl/contrib/opensolaris/uts/common CFLAGS+= -DNEED_SOLARIS_BOOLEAN diff --git a/sbin/bectl/bectl.c b/sbin/bectl/bectl.c index d0ef1422051b..98a9121a2cf3 100644 --- a/sbin/bectl/bectl.c +++ b/sbin/bectl/bectl.c @@ -54,7 +54,7 @@ static int bectl_cmd_rename(int argc, char *argv[]); static int bectl_cmd_unjail(int argc, char *argv[]); static int bectl_cmd_unmount(int argc, char *argv[]); -libbe_handle_t *be; +static libbe_handle_t *be; static int usage(bool explicit) @@ -108,7 +108,7 @@ static struct command_map_entry command_map[] = }; static int -get_cmd_index(char *cmd, int *index) +get_cmd_index(const char *cmd, int *index) { int map_size; @@ -127,7 +127,6 @@ get_cmd_index(char *cmd, int *index) static int bectl_cmd_activate(int argc, char *argv[]) { - char *bootenv; int err, opt; bool temp; @@ -210,7 +209,7 @@ bectl_cmd_create(int argc, char *argv[]) } else { if ((snapname = strchr(bootenv, '@')) != NULL) { *(snapname++) = '\0'; - if ((err = be_snapshot(be, (char *)be_active_path(be), + if ((err = be_snapshot(be, be_active_path(be), snapname, true, NULL)) != BE_ERR_SUCCESS) fprintf(stderr, "failed to create snapshot\n"); asprintf(&source, "%s@%s", be_active_path(be), snapname); @@ -242,7 +241,6 @@ static int bectl_cmd_export(int argc, char *argv[]) { char *bootenv; - int opt; if (argc == 1) { fprintf(stderr, "bectl export: missing boot environment name\n"); @@ -299,7 +297,6 @@ bectl_cmd_import(int argc, char *argv[]) static int bectl_cmd_add(int argc, char *argv[]) { - char *bootenv; if (argc < 2) { fprintf(stderr, "bectl add: must provide at least one path\n"); @@ -358,7 +355,7 @@ bectl_cmd_jail(int argc, char *argv[]) char *bootenv; char mnt_loc[BE_MAXPATHLEN]; char buf[BE_MAXPATHLEN*2]; - int err, jid; + int err; /* struct jail be_jail = { 0 }; */ @@ -418,7 +415,6 @@ static int bectl_cmd_list(int argc, char *argv[]) { nvlist_t *props; - char *bootenv; int opt; bool show_all_datasets, show_space, hide_headers, show_snaps; @@ -625,7 +621,7 @@ bectl_cmd_unmount(int argc, char *argv[]) int main(int argc, char *argv[]) { - char *command; + const char *command; int command_index, rc; if (argc < 2) {