diff mbox

[v2] ndctl, filter: fix "keyword 'all' is ignored" in util_<obj>_filter()

Message ID 20180426005648.13028-1-qi.fuli@jp.fujitsu.com
State New, archived
Headers show

Commit Message

QI Fuli April 26, 2018, 12:56 a.m. UTC
This is a follow up patch for commit c70adc3cf6bf ("ndctl, filter: refactor
util_<obj>_filter() to support multiple space-seperated arguments")
refactored util_<obj>_filter() to support multiple space-seperated arguments.
But, when the keyword "all" is included in space-seperated arguments,
it will be treaded as <object>'s name. This patch fixes it.

Signed-off-by: QI Fuli <qi.fuli@jp.fujitsu.com>

Change log since v1:
 - Removing the strcmp(__ident, "all") == 0 at the top of util_<obj>_filter()
 - Changing the strcmp(ident, "all") == 0 to strcmp(name, "all") == 0

---
 util/filter.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

Comments

Dan Williams April 26, 2018, 2:32 a.m. UTC | #1
On Wed, Apr 25, 2018 at 5:56 PM, QI Fuli <qi.fuli@jp.fujitsu.com> wrote:
> This is a follow up patch for commit c70adc3cf6bf ("ndctl, filter: refactor
> util_<obj>_filter() to support multiple space-seperated arguments")
> refactored util_<obj>_filter() to support multiple space-seperated arguments.
> But, when the keyword "all" is included in space-seperated arguments,
> it will be treaded as <object>'s name. This patch fixes it.
>
> Signed-off-by: QI Fuli <qi.fuli@jp.fujitsu.com>

Looks good,

Reviewed-by: Dan Williams <dan.j.williams@intel.com>
Verma, Vishal L May 2, 2018, 9:37 p.m. UTC | #2
On Thu, 2018-04-26 at 09:56 +0900, QI Fuli wrote:
> This is a follow up patch for commit c70adc3cf6bf ("ndctl, filter:
> refactor
> util_<obj>_filter() to support multiple space-seperated arguments")
> refactored util_<obj>_filter() to support multiple space-seperated
> arguments.
> But, when the keyword "all" is included in space-seperated arguments,
> it will be treaded as <object>'s name. This patch fixes it.
> 
> Signed-off-by: QI Fuli <qi.fuli@jp.fujitsu.com>
> 
> Change log since v1:
>  - Removing the strcmp(__ident, "all") == 0 at the top of
> util_<obj>_filter()
>  - Changing the strcmp(ident, "all") == 0 to strcmp(name, "all") == 0
> 
> ---
>  util/filter.c | 20 ++++++++++++++++----
>  1 file changed, 16 insertions(+), 4 deletions(-)

Thanks, applied. Just as a future note, you should have the "changes since
v1" notes below the '---' marker (between the --- and the first file
diffstat). This prevents it from being picked up as part of the commit
message.

Thanks,
	-Vishal

> 
> diff --git a/util/filter.c b/util/filter.c
> index 0d3cc02..1734bce 100644
> --- a/util/filter.c
> +++ b/util/filter.c
> @@ -31,7 +31,7 @@ struct ndctl_bus *util_bus_filter(struct ndctl_bus
> *bus, const char *__ident)
>  	unsigned long bus_id, id;
>  	const char *provider, *devname, *name;
>  
> -	if (!__ident || strcmp(__ident, "all") == 0)
> +	if (!__ident)
>  		return bus;
>  
>  	ident = strdup(__ident);
> @@ -40,6 +40,9 @@ struct ndctl_bus *util_bus_filter(struct ndctl_bus
> *bus, const char *__ident)
>  
>  	for (name = strtok_r(ident, " ", &save); name;
>  			name = strtok_r(NULL, " ", &save)) {
> +		if (strcmp(name, "all") == 0)
> +			break;
> +
>  		bus_id = strtoul(ident, &end, 0);
>  		if (end == ident || end[0])
>  			bus_id = ULONG_MAX;
> @@ -69,7 +72,7 @@ struct ndctl_region *util_region_filter(struct
> ndctl_region *region,
>  	const char *name, *region_name;
>  	unsigned long region_id, id;
>  
> -	if (!__ident || strcmp(__ident, "all") == 0)
> +	if (!__ident)
>  		return region;
>  
>  	ident = strdup(__ident);
> @@ -78,6 +81,9 @@ struct ndctl_region *util_region_filter(struct
> ndctl_region *region,
>  
>  	for (name = strtok_r(ident, " ", &save); name;
>  			name = strtok_r(NULL, " ", &save)) {
> +		if (strcmp(name, "all") == 0)
> +			break;
> +
>  		region_id = strtoul(ident, &end, 0);
>  		if (end == ident || end[0])
>  			region_id = ULONG_MAX;
> @@ -106,7 +112,7 @@ struct ndctl_namespace *util_namespace_filter(struct
> ndctl_namespace *ndns,
>  	const char *name;
>  	char *ident, *save;
>  
> -	if (!__ident || strcmp(__ident, "all") == 0)
> +	if (!__ident)
>  		return ndns;
>  
>  	ident = strdup(__ident);
> @@ -115,6 +121,9 @@ struct ndctl_namespace *util_namespace_filter(struct
> ndctl_namespace *ndns,
>  
>  	for (name = strtok_r(ident, " ", &save); name;
>  			name = strtok_r(NULL, " ", &save)) {
> +		if (strcmp(name, "all") == 0)
> +			break;
> +
>  		if (strcmp(name, ndctl_namespace_get_devname(ndns)) ==
> 0)
>  			break;
>  
> @@ -137,7 +146,7 @@ struct ndctl_dimm *util_dimm_filter(struct ndctl_dimm
> *dimm,
>  	const char *name, *dimm_name;
>  	unsigned long dimm_id, id;
>  
> -	if (!__ident || strcmp(__ident, "all") == 0)
> +	if (!__ident)
>  		return dimm;
>  
>  	ident = strdup(__ident);
> @@ -146,6 +155,9 @@ struct ndctl_dimm *util_dimm_filter(struct ndctl_dimm
> *dimm,
>  
>  	for (name = strtok_r(ident, " ", &save); name;
>  			name = strtok_r(NULL, " ", &save)) {
> +		if (strcmp(name, "all") == 0)
> +			break;
> +
>  		dimm_id = strtoul(ident, &end, 0);
>  		if (end == ident || end[0])
>  			dimm_id = ULONG_MAX;
QI Fuli May 7, 2018, 1:21 a.m. UTC | #3
> -----Original Message-----

> From: Verma, Vishal L [mailto:vishal.l.verma@intel.com]

> Sent: Thursday, May 3, 2018 6:38 AM

> To: linux-nvdimm@lists.01.org; Qi, Fuli/斉 福利 <qi.fuli@jp.fujitsu.com>

> Subject: Re: [PATCH v2] ndctl, filter: fix "keyword 'all' is ignored" in

> util_<obj>_filter()

> 

> On Thu, 2018-04-26 at 09:56 +0900, QI Fuli wrote:

> > This is a follow up patch for commit c70adc3cf6bf ("ndctl, filter:

> > refactor

> > util_<obj>_filter() to support multiple space-seperated arguments")

> > refactored util_<obj>_filter() to support multiple space-seperated

> > arguments.

> > But, when the keyword "all" is included in space-seperated arguments,

> > it will be treaded as <object>'s name. This patch fixes it.

> >

> > Signed-off-by: QI Fuli <qi.fuli@jp.fujitsu.com>

> >

> > Change log since v1:

> >  - Removing the strcmp(__ident, "all") == 0 at the top of

> > util_<obj>_filter()

> >  - Changing the strcmp(ident, "all") == 0 to strcmp(name, "all") == 0

> >

> > ---

> >  util/filter.c | 20 ++++++++++++++++----

> >  1 file changed, 16 insertions(+), 4 deletions(-)

> 

> Thanks, applied. Just as a future note, you should have the "changes since v1"

> notes below the '---' marker (between the --- and the first file diffstat). This

> prevents it from being picked up as part of the commit message.

> 

> Thanks,

> 	-Vishal

> 


Thank you for your comment.
I will take an eye on it.

Best,
 - Qi

> >

> > diff --git a/util/filter.c b/util/filter.c index 0d3cc02..1734bce

> > 100644

> > --- a/util/filter.c

> > +++ b/util/filter.c

> > @@ -31,7 +31,7 @@ struct ndctl_bus *util_bus_filter(struct ndctl_bus

> > *bus, const char *__ident)

> >  	unsigned long bus_id, id;

> >  	const char *provider, *devname, *name;

> >

> > -	if (!__ident || strcmp(__ident, "all") == 0)

> > +	if (!__ident)

> >  		return bus;

> >

> >  	ident = strdup(__ident);

> > @@ -40,6 +40,9 @@ struct ndctl_bus *util_bus_filter(struct ndctl_bus

> > *bus, const char *__ident)

> >

> >  	for (name = strtok_r(ident, " ", &save); name;

> >  			name = strtok_r(NULL, " ", &save)) {

> > +		if (strcmp(name, "all") == 0)

> > +			break;

> > +

> >  		bus_id = strtoul(ident, &end, 0);

> >  		if (end == ident || end[0])

> >  			bus_id = ULONG_MAX;

> > @@ -69,7 +72,7 @@ struct ndctl_region *util_region_filter(struct

> > ndctl_region *region,

> >  	const char *name, *region_name;

> >  	unsigned long region_id, id;

> >

> > -	if (!__ident || strcmp(__ident, "all") == 0)

> > +	if (!__ident)

> >  		return region;

> >

> >  	ident = strdup(__ident);

> > @@ -78,6 +81,9 @@ struct ndctl_region *util_region_filter(struct

> > ndctl_region *region,

> >

> >  	for (name = strtok_r(ident, " ", &save); name;

> >  			name = strtok_r(NULL, " ", &save)) {

> > +		if (strcmp(name, "all") == 0)

> > +			break;

> > +

> >  		region_id = strtoul(ident, &end, 0);

> >  		if (end == ident || end[0])

> >  			region_id = ULONG_MAX;

> > @@ -106,7 +112,7 @@ struct ndctl_namespace

> > *util_namespace_filter(struct ndctl_namespace *ndns,

> >  	const char *name;

> >  	char *ident, *save;

> >

> > -	if (!__ident || strcmp(__ident, "all") == 0)

> > +	if (!__ident)

> >  		return ndns;

> >

> >  	ident = strdup(__ident);

> > @@ -115,6 +121,9 @@ struct ndctl_namespace

> > *util_namespace_filter(struct ndctl_namespace *ndns,

> >

> >  	for (name = strtok_r(ident, " ", &save); name;

> >  			name = strtok_r(NULL, " ", &save)) {

> > +		if (strcmp(name, "all") == 0)

> > +			break;

> > +

> >  		if (strcmp(name, ndctl_namespace_get_devname(ndns)) ==

> > 0)

> >  			break;

> >

> > @@ -137,7 +146,7 @@ struct ndctl_dimm *util_dimm_filter(struct

> > ndctl_dimm *dimm,

> >  	const char *name, *dimm_name;

> >  	unsigned long dimm_id, id;

> >

> > -	if (!__ident || strcmp(__ident, "all") == 0)

> > +	if (!__ident)

> >  		return dimm;

> >

> >  	ident = strdup(__ident);

> > @@ -146,6 +155,9 @@ struct ndctl_dimm *util_dimm_filter(struct

> > ndctl_dimm *dimm,

> >

> >  	for (name = strtok_r(ident, " ", &save); name;

> >  			name = strtok_r(NULL, " ", &save)) {

> > +		if (strcmp(name, "all") == 0)

> > +			break;

> > +

> >  		dimm_id = strtoul(ident, &end, 0);

> >  		if (end == ident || end[0])

> >  			dimm_id = ULONG_MAX;
diff mbox

Patch

diff --git a/util/filter.c b/util/filter.c
index 0d3cc02..1734bce 100644
--- a/util/filter.c
+++ b/util/filter.c
@@ -31,7 +31,7 @@  struct ndctl_bus *util_bus_filter(struct ndctl_bus *bus, const char *__ident)
 	unsigned long bus_id, id;
 	const char *provider, *devname, *name;
 
-	if (!__ident || strcmp(__ident, "all") == 0)
+	if (!__ident)
 		return bus;
 
 	ident = strdup(__ident);
@@ -40,6 +40,9 @@  struct ndctl_bus *util_bus_filter(struct ndctl_bus *bus, const char *__ident)
 
 	for (name = strtok_r(ident, " ", &save); name;
 			name = strtok_r(NULL, " ", &save)) {
+		if (strcmp(name, "all") == 0)
+			break;
+
 		bus_id = strtoul(ident, &end, 0);
 		if (end == ident || end[0])
 			bus_id = ULONG_MAX;
@@ -69,7 +72,7 @@  struct ndctl_region *util_region_filter(struct ndctl_region *region,
 	const char *name, *region_name;
 	unsigned long region_id, id;
 
-	if (!__ident || strcmp(__ident, "all") == 0)
+	if (!__ident)
 		return region;
 
 	ident = strdup(__ident);
@@ -78,6 +81,9 @@  struct ndctl_region *util_region_filter(struct ndctl_region *region,
 
 	for (name = strtok_r(ident, " ", &save); name;
 			name = strtok_r(NULL, " ", &save)) {
+		if (strcmp(name, "all") == 0)
+			break;
+
 		region_id = strtoul(ident, &end, 0);
 		if (end == ident || end[0])
 			region_id = ULONG_MAX;
@@ -106,7 +112,7 @@  struct ndctl_namespace *util_namespace_filter(struct ndctl_namespace *ndns,
 	const char *name;
 	char *ident, *save;
 
-	if (!__ident || strcmp(__ident, "all") == 0)
+	if (!__ident)
 		return ndns;
 
 	ident = strdup(__ident);
@@ -115,6 +121,9 @@  struct ndctl_namespace *util_namespace_filter(struct ndctl_namespace *ndns,
 
 	for (name = strtok_r(ident, " ", &save); name;
 			name = strtok_r(NULL, " ", &save)) {
+		if (strcmp(name, "all") == 0)
+			break;
+
 		if (strcmp(name, ndctl_namespace_get_devname(ndns)) == 0)
 			break;
 
@@ -137,7 +146,7 @@  struct ndctl_dimm *util_dimm_filter(struct ndctl_dimm *dimm,
 	const char *name, *dimm_name;
 	unsigned long dimm_id, id;
 
-	if (!__ident || strcmp(__ident, "all") == 0)
+	if (!__ident)
 		return dimm;
 
 	ident = strdup(__ident);
@@ -146,6 +155,9 @@  struct ndctl_dimm *util_dimm_filter(struct ndctl_dimm *dimm,
 
 	for (name = strtok_r(ident, " ", &save); name;
 			name = strtok_r(NULL, " ", &save)) {
+		if (strcmp(name, "all") == 0)
+			break;
+
 		dimm_id = strtoul(ident, &end, 0);
 		if (end == ident || end[0])
 			dimm_id = ULONG_MAX;