Re: [PATCH 19/35] fabrics: use "const char *" in struct config

Hannes Reinecke
2021-02-04T07:20:20Z

On 1/26/21 9:33 PM, mwilck@suse.com wrote:
> From: Martin Wilck <mwilck@suse.com>
>
> This is easily done, and allows passing in some const char* pointers
> that would otherwise need to be strdup()d first.
>
> constifying cfg->device requires changes in the ctrl_instance()
> function, because basename() can't be used.
>
> Signed-off-by: Martin Wilck <mwilck@suse.com>
> ---
> fabrics.c | 23 ++++++++++++++---------
> fabrics.h | 17 +++++++++--------
> 2 files changed, 23 insertions(+), 17 deletions(-)
>
> diff --git a/fabrics.c b/fabrics.c
> index c1a4bb5..e3d2a3a 100644
> --- a/fabrics.c
> +++ b/fabrics.c
> @@ -252,17 +252,22 @@ empty_field:
> return strdup("\0");
> }
>
> -static int ctrl_instance(char *device)
> +int ctrl_instance(const char *device)
> {
> char d[64];
> + const char *p;
> int ret, instance;
>
> - device = basename(device);
> - ret = sscanf(device, "nvme%d", &instance);
> + p = strrchr(device, '/');
> + if (p == NULL)
> + p = device;
> + else
> + p++;
> + ret = sscanf(p, "nvme%d", &instance);
> if (ret <= 0)
> return -EINVAL;
> if (snprintf(d, sizeof(d), "nvme%d", instance) <= 0 ||
> - strcmp(device, d))
> + strcmp(p, d))
> return -EINVAL;
> return instance;
> }
> @@ -273,7 +278,7 @@ static int ctrl_instance(char *device)
> * given.
> * Return true/false based on whether it matches
> */
> -static bool ctrl_matches_connectargs(char *name, struct connect_args *args)
> +static bool ctrl_matches_connectargs(const char *name, struct connect_args *args)
> {
> struct connect_args cargs;
> bool found = false;
> @@ -815,7 +820,7 @@ add_int_argument(char **argstr, int *max_len, char *arg_str, int arg,
> }
>
> static int
> -add_argument(char **argstr, int *max_len, char *arg_str, char *arg)
> +add_argument(char **argstr, int *max_len, char *arg_str, const char *arg)
> {
> int len;
>
> @@ -1541,7 +1546,7 @@ static int scan_sys_nvme_filter(const struct dirent *d)
> /*
> * Returns 1 if disconnect occurred, 0 otherwise.
> */
> -static int disconnect_subsys(char *nqn, char *ctrl)
> +static int disconnect_subsys(const char *nqn, char *ctrl)
> {
> char *sysfs_nqn_path = NULL, *sysfs_del_path = NULL;
> char subsysnqn[NVMF_NQN_SIZE] = {};
> @@ -1579,7 +1584,7 @@ static int disconnect_subsys(char *nqn, char *ctrl)
> /*
> * Returns the number of controllers successfully disconnected.
> */
> -static int disconnect_by_nqn(char *nqn)
> +static int disconnect_by_nqn(const char *nqn)
> {
> struct dirent **devices = NULL;
> int i, n, ret = 0;
> @@ -1601,7 +1606,7 @@ static int disconnect_by_nqn(char *nqn)
> return ret;
> }
>
> -static int disconnect_by_device(char *device)
> +static int disconnect_by_device(const char *device)
> {
> int instance;
>
> diff --git a/fabrics.h b/fabrics.h
> index ce965a3..1dfbd67 100644
> --- a/fabrics.h
> +++ b/fabrics.h
> @@ -13,13 +13,13 @@ extern int fabrics_disconnect_all(const char *desc, int argc, char **argv);
> /* Symbols used by monitor.c */
>
> struct config {
> - char *nqn;
> - char *transport;
> - char *traddr;
> - char *trsvcid;
> - char *host_traddr;
> - char *hostnqn;
> - char *hostid;
> + const char *nqn;
> + const char *transport;
> + const char *traddr;
> + const char *trsvcid;
> + const char *host_traddr;
> + const char *hostnqn;
> + const char *hostid;
> int nr_io_queues;
> int nr_write_queues;
> int nr_poll_queues;
> @@ -29,7 +29,7 @@ struct config {
> int ctrl_loss_tmo;
> int tos;
> char *raw;
> - char *device;
> + const char *device;
> int duplicate_connect;
> int disable_sqflow;
> int hdr_digest;
> @@ -43,5 +43,6 @@ extern struct config cfg;
>
> int build_options(char *argstr, int max_len, bool discover);
> int do_discover(char *argstr, bool connect);
> +int ctrl_instance(const char *device);
>
> #endif
>
This really is independent on the monitor code, and should be moved to
the beginning of the patch series.

Other than that:

Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer