From patchwork Tue Apr 17 16:32:08 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Dan Williams X-Patchwork-Id: 10345883 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 3EBCC601D7 for ; Tue, 17 Apr 2018 16:32:13 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 27ACB27F98 for ; Tue, 17 Apr 2018 16:32:13 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 1C24028478; Tue, 17 Apr 2018 16:32:13 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-2.8 required=2.0 tests=BAYES_00,DKIM_SIGNED, MAILING_LIST_MULTI,RCVD_IN_DNSWL_NONE,T_DKIM_INVALID autolearn=ham version=3.3.1 Received: from ml01.01.org (ml01.01.org [198.145.21.10]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 6611227FAE for ; Tue, 17 Apr 2018 16:32:11 +0000 (UTC) Received: from [127.0.0.1] (localhost [IPv6:::1]) by ml01.01.org (Postfix) with ESMTP id 8805B2265A196; Tue, 17 Apr 2018 09:32:11 -0700 (PDT) X-Original-To: linux-nvdimm@lists.01.org Delivered-To: linux-nvdimm@lists.01.org Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2607:f8b0:4003:c0f::22a; helo=mail-ot0-x22a.google.com; envelope-from=dan.j.williams@intel.com; receiver=linux-nvdimm@lists.01.org Received: from mail-ot0-x22a.google.com (mail-ot0-x22a.google.com [IPv6:2607:f8b0:4003:c0f::22a]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 6D9262265A172 for ; Tue, 17 Apr 2018 09:32:09 -0700 (PDT) Received: by mail-ot0-x22a.google.com with SMTP id y46-v6so22065432otd.4 for ; Tue, 17 Apr 2018 09:32:09 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=intel-com.20150623.gappssmtp.com; s=20150623; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=U3mofNWeldTcjyLMoScewMhjoaW0cyi46L1D4btw9Kk=; b=nhppZjOG2A3rgZkruzrGEqlWhpsn0GEvdyZErkqRV2KuAFphMcAjUjXyBdfbI0KXs5 n3tUFPoBBBUZDCNekt3FytTAgCu5NN7DZ1eAn0p+KBVa2oMkc9z7Grrl2l2wtn5gCmNs C58kDE9gYEpFRp4GDByHwBOlVISAwhlCNx0gvWMh7HWdkh89j0bcXUG6menjd9RL6D41 FoZ0v/kixz0A2J41Hh6ojafNh5xFi6+sb8yVlaQE8Ps7X7wyC/onMa4/6f3rauCbc/XP N7hVFikcf4Wi8feHwBRex+4k3bQqKNXpW+SLyoCaJfUmtFXZiW/m9WbMButCVvUAE89K o+eA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=U3mofNWeldTcjyLMoScewMhjoaW0cyi46L1D4btw9Kk=; b=ZzuGfdfkYWcDK17AgGEWD8WwwGBKZeBOyn9kfzMWaH6lHvL8PTCM4VmlCZM0CuhKsl ESNdFM3MFebibId9SM3u2WtxljdD055Hdke1Aqj0OY1ZhkC7T47fNbIjUpfU/6FN9INz XkYye8sl00xySGWQStFw/gsgKJZ2oE0M6BHOsEJjFBw6k2qsm+On4rAAaqR1nqC+mg28 ipENPmbmdESsBYhHcIndQlINCWNCf2sgjxpRgPgTbDGLeuwS6bz5uOhn06vGJ835g98B 5XVvkJb8Vfoyg1g2Rt8J0j+N4LtUpM3Q1MgPIuxJiks4FeApuXYq2eHHsTh/1P3IRoCX RZNw== X-Gm-Message-State: ALQs6tD/Jj5SguGJuXfNp+7Dr+bgXU88JCG2PQrc1oZgjaNfiTn1TXaE dqg6PJg9m8d0T/3qaIUgmOZq0p4idpma47XArnkTKQ== X-Google-Smtp-Source: AIpwx4/3HtFjz2XxIbA1a+d5Znm/oRm5aicv+uq7HPup+RpJgF6vIJafQiIyzu5VTtm9KxVU/nUlNPIGNlyqor4Al70= X-Received: by 2002:a9d:7150:: with SMTP id y16-v6mr1535547otj.35.1523982728913; Tue, 17 Apr 2018 09:32:08 -0700 (PDT) MIME-Version: 1.0 Received: by 2002:a9d:2d36:0:0:0:0:0 with HTTP; Tue, 17 Apr 2018 09:32:08 -0700 (PDT) In-Reply-To: <0DEDF3B159719A448A49EF0E7B11E3222764DF5B@g01jpexmbkw01> References: <20180417063848.5585-1-qi.fuli@jp.fujitsu.com> <20180417063848.5585-2-qi.fuli@jp.fujitsu.com> <0DEDF3B159719A448A49EF0E7B11E3222764DF5B@g01jpexmbkw01> From: Dan Williams Date: Tue, 17 Apr 2018 09:32:08 -0700 Message-ID: Subject: Re: [PATCH 1/2] ndctl, util: add OPT_STRING_LIST to parse_option To: "Qi, Fuli" X-BeenThere: linux-nvdimm@lists.01.org X-Mailman-Version: 2.1.26 Precedence: list List-Id: "Linux-nvdimm developer list." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: linux-nvdimm Errors-To: linux-nvdimm-bounces@lists.01.org Sender: "Linux-nvdimm" X-Virus-Scanned: ClamAV using ClamSMTP On Tue, Apr 17, 2018 at 8:29 AM, Qi, Fuli wrote: > >> -----Original Message----- >> From: Dan Williams [mailto:dan.j.williams@intel.com] >> Sent: Tuesday, April 17, 2018 11:57 PM >> To: Qi, Fuli/斉 福利 >> Cc: linux-nvdimm >> Subject: Re: [PATCH 1/2] ndctl, util: add OPT_STRING_LIST to parse_option >> >> On Mon, Apr 16, 2018 at 11:38 PM, QI Fuli wrote: >> > This patch adds OPT_STRING_LIST to parse_option in order to support >> > multiple space seperated string objects in one option. >> > >> > Signed-off-by: QI Fuli >> > --- >> > ccan/list/list.h | 6 ++++++ >> > util/parse-options.c | 25 +++++++++++++++++++++++++ >> > util/parse-options.h | 3 +++ >> > 3 files changed, 34 insertions(+) >> > >> > diff --git a/ccan/list/list.h b/ccan/list/list.h index >> > 4d1d34e..f6c927f 100644 >> > --- a/ccan/list/list.h >> > +++ b/ccan/list/list.h >> > @@ -26,6 +26,12 @@ struct list_node >> > struct list_node *next, *prev; }; >> > >> > +struct string_list_node >> > +{ >> > + char *str; >> > + struct list_node list; >> > +}; >> > + >> > /** >> > * struct list_head - the head of a doubly-linked list >> > * @h: the list_head (containing next and prev pointers) diff --git >> > a/util/parse-options.c b/util/parse-options.c index 751c091..cac18f0 >> > 100644 >> > --- a/util/parse-options.c >> > +++ b/util/parse-options.c >> > @@ -20,6 +20,7 @@ >> > #include >> > #include >> > #include >> > +#include >> > >> > #define OPT_SHORT 1 >> > #define OPT_UNSET 2 >> > @@ -695,3 +696,27 @@ int parse_opt_verbosity_cb(const struct option *opt, >> > } >> > return 0; >> > } >> > + >> > +int parse_opt_string_list(const struct option *opt, const char *arg, >> > +int unset) { >> > + if (unset) >> > + return 0; >> > + >> > + if (!arg) >> > + return -1; >> > + >> > + struct list_head *v = opt->value; >> > + char *temp = strdup(arg); >> > + const char *deli = " "; >> > + >> > + temp = strtok(temp, deli); >> > + while (temp != NULL) { >> > + struct string_list_node *sln = malloc(sizeof(*sln)); >> > + sln->str = temp; >> > + list_add_tail(v, &sln->list); >> > + temp = strtok(NULL, deli); >> > + } >> > + >> > + free(temp); >> > + return 0; >> > +} >> >> As far as I can see we do not need to allocate a list or add this new >> OPT_STRING_LIST argument type. Just teach the util__filter() routines >> that the 'ident' argument may be a space delimited list. See the attached patch: > > Thank you for your comment. > > The OPT_STRING_LIST is copied from git. > Consider multiple arguments per option should be supported not only in > monitor and list but also in other commands, as Vishal mentioned: > "ndctl disable-namespace namespace1.0 namespace2.0 ..." In this case there's no "-n" option so the list parsing is already handled by standard shell argument parsing, i.e. the argv[] array already has the list split. > If you think this feature is not needed in other commands, I will delete > OPT_STRING_LIST and make a v2 patch. I can see OPT_STRING_LIST potentially being useful in other scenarios, but for the util__filter functions it seems an awkward fit to me. We end up doing the parsing twice. Once to parse the space separated list and again to parse each entry against the object to be filtered. In this case I think it is cleaner to handle it internally to the utility functions. It also makes the util functions more generically useful as I can now use them in scenarios where the string list is not coming from the command line. Ross noted that the attachment got swallowed by the list, so here it is pasted, please forgive any whitespace damage: } diff --git a/util/filter.c b/util/filter.c index 6ab391a81d94..c37b62c7e858 100644 --- a/util/filter.c +++ b/util/filter.c @@ -98,28 +98,38 @@ struct ndctl_namespace *util_namespace_filter(struct ndctl_namespace *ndns return NULL; } -struct ndctl_dimm *util_dimm_filter(struct ndctl_dimm *dimm, const char *ident) +struct ndctl_dimm *util_dimm_filter(struct ndctl_dimm *dimm, const char *__ident) { - char *end = NULL; - const char *name; + char *end = NULL, *ident, *save; + const char *name, *dimm_name; unsigned long dimm_id, id; - if (!ident || strcmp(ident, "all") == 0) + if (!__ident || strcmp(__ident, "all") == 0) return dimm; - dimm_id = strtoul(ident, &end, 0); - if (end == ident || end[0]) - dimm_id = ULONG_MAX; + ident = strdup(__ident); + if (!ident) + return NULL; - name = ndctl_dimm_get_devname(dimm); - id = ndctl_dimm_get_id(dimm); + for (name = strtok_r(ident, " ", &save); name; + name = strtok_r(NULL, " ", &save)) { + dimm_id = strtoul(name, &end, 0); + if (end == ident || end[0]) + dimm_id = ULONG_MAX; - if (dimm_id < ULONG_MAX && dimm_id == id) - return dimm; + dimm_name = ndctl_dimm_get_devname(dimm); + id = ndctl_dimm_get_id(dimm); - if (dimm_id == ULONG_MAX && strcmp(ident, name) == 0) - return dimm; + if (dimm_id < ULONG_MAX && dimm_id == id) + break; + + if (dimm_id == ULONG_MAX && strcmp(dimm_name, name) == 0) + break; + } + free(ident); + if (name) + return dimm; return NULL;