[PATCH V2 0/5] multipath-tools series: some cleanups and fixes

[PATCH V2 0/5] multipath-tools series: some cleanups and fixes

[Zhiqiang Liu]

When I learn and review the multipath-tools source code, I find
some cleanups and fixes.

V1->V2:
- rewrite some patches as suggested by Martin.

repo: openSUSE/multipath-tools
repo link: https://github.com/openSUSE/multipath-tools
branch: upstream-queue

ZhiqiangLiu (5):
  multipathd: adopt static char* arr in daemon_status
  multipathd: use daemon_status_msg to construct sd notify msg in
    do_sd_notify func
  libmultipath: check blist before calling MALLOC in alloc_ble_device
    func
  vector: add lower boundary check in vector_foreach_slot_after
  multipathd: remove useless memset after MALLOC in alloc_waiteri func

 libmultipath/blacklist.c |  8 +++--
 libmultipath/vector.h    |  7 +++--
 multipathd/main.c        | 66 +++++++++++++++++++---------------------
 multipathd/main.h        |  3 +-
 multipathd/waiter.c      |  8 +----
 5 files changed, 44 insertions(+), 48 deletions(-)

-- 
2.24.0.windows.2

[PATCH V2 1/5] multipathd: adopt static char* arr in daemon_status

[Zhiqiang Liu]

We adopt static char* array (demon_status_msg) in daemon_status func,
so it looks more simpler and easier to expand.

V1->V2:
- use "int" as the type of "status" (suggested by Martin)

Signed-off-by: Zhiqiang Liu <liuzhiqiang26@huawei.com>
Signed-off-by: lixiaokeng <lixiaokeng@huawei.com>
---
 multipathd/main.c | 30 +++++++++++++++---------------
 multipathd/main.h |  3 ++-
 2 files changed, 17 insertions(+), 16 deletions(-)

diff --git a/multipathd/main.c b/multipathd/main.c
index 9ec65856..62cf4ff4 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -153,24 +153,24 @@ static volatile sig_atomic_t exit_sig;
 static volatile sig_atomic_t reconfig_sig;
 static volatile sig_atomic_t log_reset_sig;

+static const char *daemon_status_msg[DAEMON_STATUS_SIZE] = {
+	[DAEMON_INIT] = "init",
+	[DAEMON_START] = "startup",
+	[DAEMON_CONFIGURE] = "configure",
+	[DAEMON_IDLE] = "idle",
+	[DAEMON_RUNNING] = "running",
+	[DAEMON_SHUTDOWN] = "shutdown",
+};
+
 const char *
 daemon_status(void)
 {
-	switch (get_running_state()) {
-	case DAEMON_INIT:
-		return "init";
-	case DAEMON_START:
-		return "startup";
-	case DAEMON_CONFIGURE:
-		return "configure";
-	case DAEMON_IDLE:
-		return "idle";
-	case DAEMON_RUNNING:
-		return "running";
-	case DAEMON_SHUTDOWN:
-		return "shutdown";
-	}
-	return NULL;
+	int status = get_running_state();
+
+	if (status < DAEMON_INIT || status >= DAEMON_STATUS_SIZE)
+		return NULL;
+
+	return daemon_status_msg[status];
 }

 /*
diff --git a/multipathd/main.h b/multipathd/main.h
index 5dff17e5..6a5592c0 100644
--- a/multipathd/main.h
+++ b/multipathd/main.h
@@ -4,12 +4,13 @@
 #define MAPGCINT 5

 enum daemon_status {
-	DAEMON_INIT,
+	DAEMON_INIT = 0,
 	DAEMON_START,
 	DAEMON_CONFIGURE,
 	DAEMON_IDLE,
 	DAEMON_RUNNING,
 	DAEMON_SHUTDOWN,
+	DAEMON_STATUS_SIZE,
 };

 struct prout_param_descriptor;
-- 
2.24.0.windows.2

[PATCH V2 2/5] multipathd: use daemon_status_msg to construct sd notify msg in do_sd_notify func

[Zhiqiang Liu]

sd_notify_status() is very similar with daemon_status(), except
DAEMON_IDLE and DAEMON_RUNNING state. As suggested by Martin,
we can create the sd notification string in a dynamic buffer,
and treat DAEMON_IDLE and DAEMON_RUNNING cases first. Then,
we can use daemon_status_msg[state] for other cases.

Signed-off-by: Zhiqiang Liu <liuzhiqiang26@huawei.com>
Signed-off-by: lixiaokeng <lixiaokeng@huawei.com>
---
 multipathd/main.c | 36 ++++++++++++++++--------------------
 1 file changed, 16 insertions(+), 20 deletions(-)

diff --git a/multipathd/main.c b/multipathd/main.c
index 62cf4ff4..4ba015bb 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -85,6 +85,7 @@

 #define FILE_NAME_SIZE 256
 #define CMDSIZE 160
+#define MSG_SIZE 64

 #define LOG_MSG(lvl, verb, pp)					\
 do {								\
@@ -177,28 +178,12 @@ daemon_status(void)
  * I love you too, systemd ...
  */
 #ifdef USE_SYSTEMD
-static const char *
-sd_notify_status(enum daemon_status state)
-{
-	switch (state) {
-	case DAEMON_INIT:
-		return "STATUS=init";
-	case DAEMON_START:
-		return "STATUS=startup";
-	case DAEMON_CONFIGURE:
-		return "STATUS=configure";
-	case DAEMON_IDLE:
-	case DAEMON_RUNNING:
-		return "STATUS=up";
-	case DAEMON_SHUTDOWN:
-		return "STATUS=shutdown";
-	}
-	return NULL;
-}
-
 static void do_sd_notify(enum daemon_status old_state,
 			 enum daemon_status new_state)
 {
+	char notify_msg[MSG_SIZE];
+	const char prefix[] = "STATUS=";
+	const char *msg = NULL;
 	/*
 	 * Checkerloop switches back and forth between idle and running state.
 	 * No need to tell systemd each time.
@@ -207,7 +192,18 @@ static void do_sd_notify(enum daemon_status old_state,
 	if ((new_state == DAEMON_IDLE || new_state == DAEMON_RUNNING) &&
 	    (old_state == DAEMON_IDLE || old_state == DAEMON_RUNNING))
 		return;
-	sd_notify(0, sd_notify_status(new_state));
+
+	if (new_state == DAEMON_IDLE || new_state == DAEMON_RUNNING)
+		msg = "up";
+	else
+		msg = daemon_status_msg[new_state];
+
+	if (snprintf(notify_msg, MSG_SIZE, "%s%s", prefix, msg) >= MSG_SIZE) {
+		condlog(2, "len of msg is should be shorter than %d", MSG_SIZE);
+		return;
+	}
+
+	sd_notify(0, notify_msg);
 }
 #endif

-- 
2.24.0.windows.2

[PATCH V2 3/5] libmultipath: check blist before calling MALLOC in alloc_ble_device func

[Zhiqiang Liu]

In alloc_ble_device func, ble is firstly allocated by calling MALLOC,
and then input blist is checked whether it is valid. If blist is not
valid, ble will be freed without using.

Here, we should check blist firstly.

Signed-off-by: Zhiqiang Liu <liuzhiqiang26@huawei.com>
Signed-off-by: lixiaokeng <lixiaokeng@huawei.com>
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 libmultipath/blacklist.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/libmultipath/blacklist.c b/libmultipath/blacklist.c
index db58ccca..bedcc7e4 100644
--- a/libmultipath/blacklist.c
+++ b/libmultipath/blacklist.c
@@ -66,12 +66,16 @@ out:

 int alloc_ble_device(vector blist)
 {
-	struct blentry_device * ble = MALLOC(sizeof(struct blentry_device));
+	struct blentry_device *ble;

+	if (!blist)
+		return 1;
+
+	ble = MALLOC(sizeof(struct blentry_device));
 	if (!ble)
 		return 1;

-	if (!blist || !vector_alloc_slot(blist)) {
+	if (!vector_alloc_slot(blist)) {
 		FREE(ble);
 		return 1;
 	}
-- 
2.24.0.windows.2

[PATCH V2 4/5] vector: add lower boundary check in vector_foreach_slot_after

[Zhiqiang Liu]

In vector_foreach_slot_after macro, i is the input var, which
may have a illegal value (i < 0). So we should add lower boundary
check in vector_foreach_slot_after macro.

V1->V2:
- check whether i is illegal before loop (As suggested by Martin)

Signed-off-by: Zhiqiang Liu <liuzhiqiang26@huawei.com>
Signed-off-by: lixiaokeng <lixiaokeng@huawei.com>
---
 libmultipath/vector.h | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/libmultipath/vector.h b/libmultipath/vector.h
index 2862dc28..ab80d06e 100644
--- a/libmultipath/vector.h
+++ b/libmultipath/vector.h
@@ -38,11 +38,12 @@ typedef struct _vector *vector;
 #define VECTOR_LAST_SLOT(V)   (((V) && VECTOR_SIZE(V) > 0) ? (V)->slot[(VECTOR_SIZE(V) - 1)] : NULL)

 #define vector_foreach_slot(v,p,i) \
-	for (i = 0; (v) && (int)i < VECTOR_SIZE(v) && ((p) = (v)->slot[i]); i++)
+	for ((i) = 0; (v) && (int)(i) < VECTOR_SIZE(v) && ((p) = (v)->slot[i]); (i)++)
 #define vector_foreach_slot_after(v,p,i) \
-	for (; (v) && (int)i < VECTOR_SIZE(v) && ((p) = (v)->slot[i]); i++)
+	if ((v) && (int)(i) >= 0) \
+		for (; (int)(i) < VECTOR_SIZE(v) && ((p) = (v)->slot[i]); (i)++)
 #define vector_foreach_slot_backwards(v,p,i) \
-	for (i = VECTOR_SIZE(v) - 1; (int)i >= 0 && ((p) = (v)->slot[i]); i--)
+	for ((i) = VECTOR_SIZE(v) - 1; (int)(i) >= 0 && ((p) = (v)->slot[i]); (i)--)

 #define identity(x) (x)
 /*
-- 
2.24.0.windows.2

[PATCH V2 5/5] multipathd: remove useless memset after MALLOC in alloc_waiteri func

[Zhiqiang Liu]

MALLOC(x) maps to calloc(1, x), which already init var to zero.
In alloc_waiter func, the superfluous memset(wp) call is useless,
and it should be removed.

V1->V2:
- remove useless memset call after MALLOC (as suggested by Martin)

Signed-off-by: Zhiqiang Liu <liuzhiqiang26@huawei.com>
Signed-off-by: lixiaokeng <lixiaokeng@huawei.com>
---
 multipathd/waiter.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/multipathd/waiter.c b/multipathd/waiter.c
index e6457663..da4017b9 100644
--- a/multipathd/waiter.c
+++ b/multipathd/waiter.c
@@ -29,13 +29,7 @@ struct mutex_lock waiter_lock = { .mutex = PTHREAD_MUTEX_INITIALIZER };

 static struct event_thread *alloc_waiter (void)
 {
-
-	struct event_thread *wp;
-
-	wp = (struct event_thread *)MALLOC(sizeof(struct event_thread));
-	memset(wp, 0, sizeof(struct event_thread));
-
-	return wp;
+	return (struct event_thread *)MALLOC(sizeof(struct event_thread));
 }

 static void free_waiter (void *data)
-- 
2.24.0.windows.2

Re: [PATCH V2 1/5] multipathd: adopt static char* arr in daemon_status

[Martin Wilck]

On Fri, 2020-08-21 at 18:58 +0800, Zhiqiang Liu wrote:
> We adopt static char* array (demon_status_msg) in daemon_status func,
> so it looks more simpler and easier to expand.
> 
> V1->V2:
> - use "int" as the type of "status" (suggested by Martin)
> 

Reviewed-by: Martin Wilck <mwilck@suse.com>

> Signed-off-by: Zhiqiang Liu <liuzhiqiang26@huawei.com>
> Signed-off-by: lixiaokeng <lixiaokeng@huawei.com>
> ---
>  multipathd/main.c | 30 +++++++++++++++---------------
>  multipathd/main.h |  3 ++-
>  2 files changed, 17 insertions(+), 16 deletions(-)

Re: [PATCH V2 2/5] multipathd: use daemon_status_msg to construct sd notify msg in do_sd_notify func

[Martin Wilck]

On Fri, 2020-08-21 at 18:59 +0800, Zhiqiang Liu wrote:
> sd_notify_status() is very similar with daemon_status(), except
> DAEMON_IDLE and DAEMON_RUNNING state. As suggested by Martin,
> we can create the sd notification string in a dynamic buffer,
> and treat DAEMON_IDLE and DAEMON_RUNNING cases first. Then,
> we can use daemon_status_msg[state] for other cases.
> 
> Signed-off-by: Zhiqiang Liu <liuzhiqiang26@huawei.com>
> Signed-off-by: lixiaokeng <lixiaokeng@huawei.com>
> ---
>  multipathd/main.c | 36 ++++++++++++++++--------------------
>  1 file changed, 16 insertions(+), 20 deletions(-)
> 
> diff --git a/multipathd/main.c b/multipathd/main.c
> index 62cf4ff4..4ba015bb 100644
> --- a/multipathd/main.c
> +++ b/multipathd/main.c
> @@ -85,6 +85,7 @@
> 
>  #define FILE_NAME_SIZE 256
>  #define CMDSIZE 160
> +#define MSG_SIZE 64
> 
>  #define LOG_MSG(lvl, verb, pp)					
> \
>  do {								\
> @@ -177,28 +178,12 @@ daemon_status(void)
>   * I love you too, systemd ...
>   */
>  #ifdef USE_SYSTEMD
> -static const char *
> -sd_notify_status(enum daemon_status state)
> -{
> -	switch (state) {
> -	case DAEMON_INIT:
> -		return "STATUS=init";
> -	case DAEMON_START:
> -		return "STATUS=startup";
> -	case DAEMON_CONFIGURE:
> -		return "STATUS=configure";
> -	case DAEMON_IDLE:
> -	case DAEMON_RUNNING:
> -		return "STATUS=up";
> -	case DAEMON_SHUTDOWN:
> -		return "STATUS=shutdown";
> -	}
> -	return NULL;
> -}
> -
>  static void do_sd_notify(enum daemon_status old_state,
>  			 enum daemon_status new_state)
>  {
> +	char notify_msg[MSG_SIZE];
> +	const char prefix[] = "STATUS=";
> +	const char *msg = NULL;
>  	/*
>  	 * Checkerloop switches back and forth between idle and running
> state.
>  	 * No need to tell systemd each time.
> @@ -207,7 +192,18 @@ static void do_sd_notify(enum daemon_status
> old_state,
>  	if ((new_state == DAEMON_IDLE || new_state == DAEMON_RUNNING)
> &&
>  	    (old_state == DAEMON_IDLE || old_state == DAEMON_RUNNING))
>  		return;
> -	sd_notify(0, sd_notify_status(new_state));
> +
> +	if (new_state == DAEMON_IDLE || new_state == DAEMON_RUNNING)
> +		msg = "up";
> +	else
> +		msg = daemon_status_msg[new_state];
> +
> +	if (snprintf(notify_msg, MSG_SIZE, "%s%s", prefix, msg) >=
> MSG_SIZE) {
> +		condlog(2, "len of msg is should be shorter than %d",
> MSG_SIZE);

You can *prove* that this condition can never occur (actually, you'll
never need more than 17 bytes - it's ok to have some head room). The
compiler may force you to check the return value, but it's safe to
write simply

if (!safe_sprintf(notify_msg, "%s%s", prefix, msg))
       sd_notify(0, notify_msg);

No error message is necessary here.

Martin

> +		return;
> +	}
> +
> +	sd_notify(0, notify_msg);
>  }
>  #endif
> 

Re: [PATCH V2 3/5] libmultipath: check blist before calling MALLOC in alloc_ble_device func

[Martin Wilck]

On Fri, 2020-08-21 at 19:00 +0800, Zhiqiang Liu wrote:
> In alloc_ble_device func, ble is firstly allocated by calling MALLOC,
> and then input blist is checked whether it is valid. If blist is not
> valid, ble will be freed without using.
> 
> Here, we should check blist firstly.
> 
> Signed-off-by: Zhiqiang Liu <liuzhiqiang26@huawei.com>
> Signed-off-by: lixiaokeng <lixiaokeng@huawei.com>
> Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>

Reviewed-by: Martin Wilck <mwilck@suse.com>

This one is already applied in my "upstream-queue" branch.

Re: [PATCH V2 5/5] multipathd: remove useless memset after MALLOC in alloc_waiteri func

[Martin Wilck]

On Fri, 2020-08-21 at 19:01 +0800, Zhiqiang Liu wrote:
> MALLOC(x) maps to calloc(1, x), which already init var to zero.
> In alloc_waiter func, the superfluous memset(wp) call is useless,
> and it should be removed.
> 
> V1->V2:
> - remove useless memset call after MALLOC (as suggested by Martin)

Reading this, I realize that we don't need alloc_waiter() at all.
Anyway:

Reviewed-by: Martin Wilck <mwilck@suse.com>

> Signed-off-by: Zhiqiang Liu <liuzhiqiang26@huawei.com>
> Signed-off-by: lixiaokeng <lixiaokeng@huawei.com>
> ---
>  multipathd/waiter.c | 8 +-------
>  1 file changed, 1 insertion(+), 7 deletions(-)
> 
> diff --git a/multipathd/waiter.c b/multipathd/waiter.c
> index e6457663..da4017b9 100644
> --- a/multipathd/waiter.c
> +++ b/multipathd/waiter.c
> @@ -29,13 +29,7 @@ struct mutex_lock waiter_lock = { .mutex =
> PTHREAD_MUTEX_INITIALIZER };
> 
>  static struct event_thread *alloc_waiter (void)
>  {
> -
> -	struct event_thread *wp;
> -
> -	wp = (struct event_thread *)MALLOC(sizeof(struct
> event_thread));
> -	memset(wp, 0, sizeof(struct event_thread));
> -
> -	return wp;
> +	return (struct event_thread *)MALLOC(sizeof(struct
> event_thread));
>  }
> 
>  static void free_waiter (void *data)

Re: [PATCH V2 4/5] vector: add lower boundary check in vector_foreach_slot_after

[Martin Wilck]

On Fri, 2020-08-21 at 19:00 +0800, Zhiqiang Liu wrote:
> In vector_foreach_slot_after macro, i is the input var, which
> may have a illegal value (i < 0). So we should add lower boundary
> check in vector_foreach_slot_after macro.
> 
> V1->V2:
> - check whether i is illegal before loop (As suggested by Martin)
> 

Reviewed-by: Martin Wilck <mwilck@suse.com>

> Signed-off-by: Zhiqiang Liu <liuzhiqiang26@huawei.com>
> Signed-off-by: lixiaokeng <lixiaokeng@huawei.com>
> ---
>  libmultipath/vector.h | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/libmultipath/vector.h b/libmultipath/vector.h
> index 2862dc28..ab80d06e 100644
> --- a/libmultipath/vector.h
> +++ b/libmultipath/vector.h
> @@ -38,11 +38,12 @@ typedef struct _vector *vector;
>  #define VECTOR_LAST_SLOT(V)   (((V) && VECTOR_SIZE(V) > 0) ? (V)-
> >slot[(VECTOR_SIZE(V) - 1)] : NULL)
> 
>  #define vector_foreach_slot(v,p,i) \
> -	for (i = 0; (v) && (int)i < VECTOR_SIZE(v) && ((p) = (v)-
> >slot[i]); i++)
> +	for ((i) = 0; (v) && (int)(i) < VECTOR_SIZE(v) && ((p) = (v)-
> >slot[i]); (i)++)
>  #define vector_foreach_slot_after(v,p,i) \
> -	for (; (v) && (int)i < VECTOR_SIZE(v) && ((p) = (v)->slot[i]);
> i++)
> +	if ((v) && (int)(i) >= 0) \
> +		for (; (int)(i) < VECTOR_SIZE(v) && ((p) = (v)-
> >slot[i]); (i)++)
>  #define vector_foreach_slot_backwards(v,p,i) \
> -	for (i = VECTOR_SIZE(v) - 1; (int)i >= 0 && ((p) = (v)-
> >slot[i]); i--)
> +	for ((i) = VECTOR_SIZE(v) - 1; (int)(i) >= 0 && ((p) = (v)-
> >slot[i]); (i)--)
> 
>  #define identity(x) (x)
>  /*

Re: [PATCH V2 1/5] multipathd: adopt static char* arr in daemon_status

[Martin Wilck]

On Fri, 2020-08-21 at 18:58 +0800, Zhiqiang Liu wrote:
> We adopt static char* array (demon_status_msg) in daemon_status func,
> so it looks more simpler and easier to expand.
> 
> V1->V2:
> - use "int" as the type of "status" (suggested by Martin)
> 
> Signed-off-by: Zhiqiang Liu <liuzhiqiang26@huawei.com>
> Signed-off-by: lixiaokeng <lixiaokeng@huawei.com>
> ---
>  multipathd/main.c | 30 +++++++++++++++---------------
>  multipathd/main.h |  3 ++-
>  2 files changed, 17 insertions(+), 16 deletions(-)
> 
> diff --git a/multipathd/main.c b/multipathd/main.c
> index 9ec65856..62cf4ff4 100644
> --- a/multipathd/main.c
> +++ b/multipathd/main.c
> diff --git a/multipathd/main.h b/multipathd/main.h
> index 5dff17e5..6a5592c0 100644
> --- a/multipathd/main.h
> +++ b/multipathd/main.h
> @@ -4,12 +4,13 @@
>  #define MAPGCINT 5
> 
>  enum daemon_status {
> -	DAEMON_INIT,
> +	DAEMON_INIT = 0,
>  	DAEMON_START,
>  	DAEMON_CONFIGURE,
>  	DAEMON_IDLE,
>  	DAEMON_RUNNING,
>  	DAEMON_SHUTDOWN,
> +	DAEMON_STATUS_SIZE,
>  };

This breaks compilation:

main.c: In function ‘sd_notify_status’:
main.c:184:2: error: enumeration value ‘DAEMON_STATUS_SIZE’ not handled
in switch [-Werror=switch]
  switch (state) {
  ^~~~~~

Please avoid introducing DAEMON_STATUS_SIZE.

This would be fixed by your patch 2 because it removes the switch
statement, but no patch in a series should break compilation, to 
allow future bisections.

I have to withdraw my "Reviewed-by: for the time being, sorry.

Martin


> 
>  struct prout_param_descriptor;



--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Re: [PATCH V2 1/5] multipathd: adopt static char* arr in daemon_status

[Zhiqiang Liu]

On 2020/8/21 21:42, Martin Wilck wrote:
> On Fri, 2020-08-21 at 18:58 +0800, Zhiqiang Liu wrote:
>> We adopt static char* array (demon_status_msg) in daemon_status func,
>> so it looks more simpler and easier to expand.
>>
>> V1->V2:
>> - use "int" as the type of "status" (suggested by Martin)
>>
>> Signed-off-by: Zhiqiang Liu <liuzhiqiang26@huawei.com>
>> Signed-off-by: lixiaokeng <lixiaokeng@huawei.com>
>> ---
>>  multipathd/main.c | 30 +++++++++++++++---------------
>>  multipathd/main.h |  3 ++-
>>  2 files changed, 17 insertions(+), 16 deletions(-)
>>
>> diff --git a/multipathd/main.c b/multipathd/main.c
>> index 9ec65856..62cf4ff4 100644
>> --- a/multipathd/main.c
>> +++ b/multipathd/main.c
>> diff --git a/multipathd/main.h b/multipathd/main.h
>> index 5dff17e5..6a5592c0 100644
>> --- a/multipathd/main.h
>> +++ b/multipathd/main.h
>> @@ -4,12 +4,13 @@
>>  #define MAPGCINT 5
>>
>>  enum daemon_status {
>> -	DAEMON_INIT,
>> +	DAEMON_INIT = 0,
>>  	DAEMON_START,
>>  	DAEMON_CONFIGURE,
>>  	DAEMON_IDLE,
>>  	DAEMON_RUNNING,
>>  	DAEMON_SHUTDOWN,
>> +	DAEMON_STATUS_SIZE,
>>  };
> 
> This breaks compilation:
> 
> main.c: In function ‘sd_notify_status’:
> main.c:184:2: error: enumeration value ‘DAEMON_STATUS_SIZE’ not handled
> in switch [-Werror=switch]
>   switch (state) {
>   ^~~~~~
> 
> Please avoid introducing DAEMON_STATUS_SIZE.
> 
> This would be fixed by your patch 2 because it removes the switch
> statement, but no patch in a series should break compilation, to 
> allow future bisections.
> 
> I have to withdraw my "Reviewed-by: for the time being, sorry.

> Martin

Thanks for your reply.
I think we should keep DAEMON_STATUS_SIZE for boundary check.
For solving the problem, we can add default case in switch(state)
in sd_notify_status func.

I will send the v3 patch series.

Regards
Zhiqiang Liu
> 
> 
>>
>>  struct prout_param_descriptor;
> 
> 
> 
> .
> 


--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel