[dm-devel] [PATCH 3/6] multipathd: adopt static char* arr in sd_notify_status func

[Zhiqiang Liu]

On 2020/8/17 16:35, Martin Wilck wrote:
> On Sun, 2020-08-16 at 09:44 +0800, Zhiqiang Liu wrote:
>> We adopt static char* array (sd_notify_status_msg) in
>> sd_notify_status func, so it looks more simpler and easier
>> to expand.
>>
>> Signed-off-by: Zhiqiang Liu <liuzhiqiang26 at huawei.com>
>> Signed-off-by: lixiaokeng <lixiaokeng at huawei.com>
>> ---
>>  multipathd/main.c | 26 ++++++++++++--------------
>>  1 file changed, 12 insertions(+), 14 deletions(-)
>>
>> diff --git a/multipathd/main.c b/multipathd/main.c
>> index cab1d0d..a09ccd1 100644
>> --- a/multipathd/main.c
>> +++ b/multipathd/main.c
>> @@ -177,23 +177,21 @@ daemon_status(void)
>>   * I love you too, systemd ...
>>   */
>>  #ifdef USE_SYSTEMD
>> +static const char *sd_notify_status_msg[DAEMON_STATUS_SIZE] = {
>> +	[DAEMON_INIT] = "STATUS=init",
>> +	[DAEMON_START] = "STATUS=startup",
>> +	[DAEMON_CONFIGURE] = "STATUS=configure",
>> +	[DAEMON_IDLE] = "STATUS=up",
>> +	[DAEMON_RUNNING] = "STATUS=up",
>> +	[DAEMON_SHUTDOWN] = "STATUS=shutdown",
>> +};
>> +
> 
> This repetition of "STATUS=" looks clumsy. It's not your fault, because
> the current code does the same thing. But if you want to clean this up,
> please create the notification string in a dynamic buffer, and use
> daemon_status() for those cases where it applies.
> 
> Regards
> Martin
> 
Thanks for your reply.
Besides the prefixes "STATUS=", there are also some differences between
sd_notify_status_msg[DAEMON_IDLE|DAEMON_RUNNING] and
demon_status_msg[DAEMON_IDLE|DAEMON_RUNNING].

For example,
sd_notify_status_msg[DAEMON_RUNNING] = "STATUS=up",
demon_status_msg[DAEMON_RUNNING] = "running"

So if we create the notification string in a dynamic buffer with
using daemon_status, we have to make some special judgement
on DAEMON_RUNNING and DAEMON_IDLE status. This may be why
the sd_notify_status func was created.

We can implement both solutions. Martin, which one do you prefer?


Regards
Zhiqiang Liu


>>  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;
>> +	if (state < DAEMON_INIT || state >= DAEMON_STATUS_SIZE)
>> +		return NULL;
>> +	return sd_notify_status_msg[state];
>>  }
>>
>>  static void do_sd_notify(enum daemon_status old_state,
> 
> 
> 
> .
>