[dm-devel] [PATCH] multipathd: fix issue in 'map $map getprstatus' reply

[peng.liang5 at zte.com.cn]

Hello Bart,

I can understand your concern. It may be some other error not just memory 
error
if asprintf() fails, so returning ENOMEM is not accurate.
Additionally, you still need to verify the numeric value of ETIMEDOUT 
because 
cli_getprstatus() returning 1 in the other code as follows.
        if (!mpp)
                return 1;
And the other called functions also returns 1 if error occurs, such as 
cli_suspend,
cli_resume.

Thanks 





发件人:         Bart Van Assche <bart.vanassche at sandisk.com>
收件人:         <peng.liang5 at zte.com.cn>, 
抄送:   <dm-devel at redhat.com>, <tang.junhui at zte.com.cn>, 
<zhang.kai16 at zte.com.cn>
日期:   2016/10/14 06:44
主题:   Re: [dm-devel] [PATCH] multipathd: fix issue in 'map $map 
getprstatus' reply



On 10/12/2016 11:30 PM, peng.liang5 at zte.com.cn wrote:
> Thanks for your attention. I don't think it is necessary to do that.
> Whatever returning 1 or ENOMEM it will reply "fail\n" and set the
> returning to 1.
>
> The executed code in uxsock_trigger as follows.
>         if (r > 0) {
>                 if (r == ETIMEDOUT)
>                         *reply = STRDUP("timeout\n");
>                 else
>                         *reply = STRDUP("fail\n");
>                 *len = strlen(*reply) + 1;
>                 r = 1;
>         }

Hello Peng,

Anyone who wants to verify your patch has to look up the numeric value 
of ETIMEDOUT to see whether or not the value of that constant is equal 
to one. So using "return 1" makes the source code harder to read than 
"return ENOMEM". Additionally, it is inconsistent from a stylistic point 
of view that the caller compares the return value with an E... error 
code and that the called function returns a numeric constant. So I would 
appreciate it very much if you would change "return 1" into "return 
ENOMEM" or any other symbolic E* error code of your preference.

Bart.


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/dm-devel/attachments/20161014/540d6135/attachment.htm>