From bcb2b73358f1c05e6b5c48cfd19e3762cc69c677 Mon Sep 17 00:00:00 2001 From: Eric Sandeen Date: Fri, 25 Jan 2013 13:27:47 -0600 Subject: [PATCH] btrfs-progs: simplify ioctl name copy and null termination In the places where we copy a string into the name member of btrfs_ioctl_vol_args or btrfs_ioctl_vol_args_v2, we use strncopy (to not overflow the name array) and then set the last position to the null character. Howver, in both cases the arrays are defined with: char name[MAX+1]; hence the last array position is name[MAX]. In most cases, we now insert the null at name[MAX-1] which deprives us of one useful character. Even the above isn't consistent through the code, so make some helper code to make it simple, i.e. strncpy_null(dest, src) which automatically does the right thing based on the size of dest. Thanks to Zach Brown for the macro suggestion. Signed-off-by: Eric Sandeen --- btrfsctl.c | 7 +++---- cmds-device.c | 9 +++------ cmds-filesystem.c | 3 +-- cmds-subvolume.c | 13 +++++-------- utils.c | 20 ++++++++++++++++++++ utils.h | 5 +++++ 6 files changed, 37 insertions(+), 20 deletions(-) diff --git a/btrfsctl.c b/btrfsctl.c index 049a5f35..8fd8cc32 100644 --- a/btrfsctl.c +++ b/btrfsctl.c @@ -242,10 +242,9 @@ int main(int ac, char **av) fd = btrfsctl_open_file_or_dir(fname); } - if (name) { - strncpy(args.name, name, BTRFS_PATH_NAME_MAX + 1); - args.name[BTRFS_PATH_NAME_MAX] = 0; - } else + if (name) + strncpy_null(args.name, name); + else args.name[0] = '\0'; if (command == BTRFS_IOC_SNAP_CREATE) { diff --git a/cmds-device.c b/cmds-device.c index 7a0f7a40..198ad689 100644 --- a/cmds-device.c +++ b/cmds-device.c @@ -116,8 +116,7 @@ static int cmd_add_dev(int argc, char **argv) } close(devfd); - strncpy(ioctl_args.name, argv[i], BTRFS_PATH_NAME_MAX); - ioctl_args.name[BTRFS_PATH_NAME_MAX-1] = 0; + strncpy_null(ioctl_args.name, argv[i]); res = ioctl(fdmnt, BTRFS_IOC_ADD_DEV, &ioctl_args); e = errno; if(res<0){ @@ -161,8 +160,7 @@ static int cmd_rm_dev(int argc, char **argv) struct btrfs_ioctl_vol_args arg; int res; - strncpy(arg.name, argv[i], BTRFS_PATH_NAME_MAX); - arg.name[BTRFS_PATH_NAME_MAX-1] = 0; + strncpy_null(arg.name, argv[i]); res = ioctl(fdmnt, BTRFS_IOC_RM_DEV, &arg); e = errno; if(res<0){ @@ -227,8 +225,7 @@ static int cmd_scan_dev(int argc, char **argv) printf("Scanning for Btrfs filesystems in '%s'\n", argv[i]); - strncpy(args.name, argv[i], BTRFS_PATH_NAME_MAX); - args.name[BTRFS_PATH_NAME_MAX-1] = 0; + strncpy_null(args.name, argv[i]); /* * FIXME: which are the error code returned by this ioctl ? * it seems that is impossible to understand if there no is diff --git a/cmds-filesystem.c b/cmds-filesystem.c index 045c896f..295592b0 100644 --- a/cmds-filesystem.c +++ b/cmds-filesystem.c @@ -478,8 +478,7 @@ static int cmd_resize(int argc, char **argv) } printf("Resize '%s' of '%s'\n", path, amount); - strncpy(args.name, amount, BTRFS_PATH_NAME_MAX); - args.name[BTRFS_PATH_NAME_MAX-1] = 0; + strncpy_null(args.name, amount); res = ioctl(fd, BTRFS_IOC_RESIZE, &args); e = errno; close(fd); diff --git a/cmds-subvolume.c b/cmds-subvolume.c index 1432b997..ea128fce 100644 --- a/cmds-subvolume.c +++ b/cmds-subvolume.c @@ -32,6 +32,7 @@ #include "ctree.h" #include "commands.h" +#include "utils.h" #include "btrfs-list.h" #include "utils.h" @@ -138,8 +139,7 @@ static int cmd_subvol_create(int argc, char **argv) struct btrfs_ioctl_vol_args_v2 args; memset(&args, 0, sizeof(args)); - strncpy(args.name, newname, BTRFS_SUBVOL_NAME_MAX); - args.name[BTRFS_SUBVOL_NAME_MAX-1] = 0; + strncpy_null(args.name, newname); args.flags |= BTRFS_SUBVOL_QGROUP_INHERIT; args.size = qgroup_inherit_size(inherit); args.qgroup_inherit = inherit; @@ -149,8 +149,7 @@ static int cmd_subvol_create(int argc, char **argv) struct btrfs_ioctl_vol_args args; memset(&args, 0, sizeof(args)); - strncpy(args.name, newname, BTRFS_PATH_NAME_MAX); - args.name[BTRFS_PATH_NAME_MAX-1] = 0; + strncpy_null(args.name, newname); res = ioctl(fddst, BTRFS_IOC_SUBVOL_CREATE, &args); } @@ -250,8 +249,7 @@ again: } printf("Delete subvolume '%s/%s'\n", dname, vname); - strncpy(args.name, vname, BTRFS_PATH_NAME_MAX); - args.name[BTRFS_PATH_NAME_MAX-1] = 0; + strncpy_null(args.name, vname); res = ioctl(fd, BTRFS_IOC_SNAP_DESTROY, &args); e = errno; @@ -597,8 +595,7 @@ static int cmd_snapshot(int argc, char **argv) args.size = qgroup_inherit_size(inherit); args.qgroup_inherit = inherit; } - strncpy(args.name, newname, BTRFS_SUBVOL_NAME_MAX); - args.name[BTRFS_SUBVOL_NAME_MAX-1] = 0; + strncpy_null(args.name, newname); res = ioctl(fddst, BTRFS_IOC_SNAP_CREATE_V2, &args); e = errno; diff --git a/utils.c b/utils.c index 7a1e39d7..f9ee8121 100644 --- a/utils.c +++ b/utils.c @@ -1125,6 +1125,26 @@ char *pretty_sizes(u64 size) return pretty; } +/* + * __strncpy__null - strncpy with null termination + * @dest: the target array + * @src: the source string + * @n: maximum bytes to copy (size of *dest) + * + * Like strncpy, but ensures destination is null-terminated. + * + * Copies the string pointed to by src, including the terminating null + * byte ('\0'), to the buffer pointed to by dest, up to a maximum + * of n bytes. Then ensure that dest is null-terminated. + */ +char *__strncpy__null(char *dest, const char *src, size_t n) +{ + strncpy(dest, src, n); + if (n > 0) + dest[n - 1] = '\0'; + return dest; +} + /* * Checks to make sure that the label matches our requirements. * Returns: diff --git a/utils.h b/utils.h index 2d2c23d2..bbcaf6a7 100644 --- a/utils.h +++ b/utils.h @@ -53,4 +53,9 @@ int get_device_info(int fd, u64 devid, struct btrfs_ioctl_dev_info_args *di_args); int get_fs_info(int fd, char *path, struct btrfs_ioctl_fs_info_args *fi_args, struct btrfs_ioctl_dev_info_args **di_ret); + +char *__strncpy__null(char *dest, const char *src, size_t n); +/* Helper to always get proper size of the destination string */ +#define strncpy_null(dest, src) __strncpy__null(dest, src, sizeof(dest)) + #endif