Re: [dm-devel] [PATCH] multipathd: fix issue in 'map $map getprstatus' reply
Hello Bart, 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; } 发件人: Bart Van Assche <bart.vanass...@sandisk.com> 收件人: <peng.lia...@zte.com.cn>, <christophe.varo...@opensvc.com>, 抄送: <tang.jun...@zte.com.cn>, <zhang.ka...@zte.com.cn>, <dm-devel@redhat.com> 日期: 2016/10/12 22:44 主题: Re: [dm-devel] [PATCH] multipathd: fix issue in 'map $map getprstatus' reply On 10/11/16 20:03, peng.lia...@zte.com.cn wrote: > From: peng liang <peng.lia...@zte.com.cn> > > -add missing newline to 'map|multipath $map getprstatus' reply > -use asprintf instead of sprintf > > Signed-off-by: peng liang <peng.lia...@zte.com.cn> > --- > multipathd/cli_handlers.c | 14 ++ > 1 file changed, 6 insertions(+), 8 deletions(-) > > diff --git a/multipathd/cli_handlers.c b/multipathd/cli_handlers.c > index 8ff4362..16445ea 100644 > --- a/multipathd/cli_handlers.c > +++ b/multipathd/cli_handlers.c > @@ -1,6 +1,9 @@ > /* > * Copyright (c) 2005 Christophe Varoqui > */ > +#define _GNU_SOURCE > + > +#include > #include "checkers.h" > #include "memory.h" > #include "vector.h" > @@ -1285,14 +1288,9 @@ cli_getprstatus (void * v, char ** reply, int * > len, void * data) > > condlog(3, "%s: prflag = %u", param, (unsigned int)mpp->prflag); > > - *reply =(char *)malloc(2); > - *len = 2; > - memset(*reply,0,2); > - > - > - sprintf(*reply,"%d",mpp->prflag); > - (*reply)[1]='\0'; > - > + *len = asprintf(reply, "%d\n", mpp->prflag); > + if (*len < 0) > + return 1; > > condlog(3, "%s: reply = %s", param, *reply); Hello Peng, Sorry but returning 1 looks somewhat inconsistent to me. This function is called indirectly by uxsock_trigger() and that function expects that cli_getprstatus() either returns a positive error code (E...) or a negative error code. Please change this patch such that ENOMEM is returned instead of 1 if asprintf() fails. Thanks, Bart. -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] [PATCH] multipathd: fix issue in 'map $map getprstatus' reply
On 10/11/16 20:03, peng.lia...@zte.com.cn wrote: > From: peng liang> > -add missing newline to 'map|multipath $map getprstatus' reply > -use asprintf instead of sprintf > > Signed-off-by: peng liang > --- > multipathd/cli_handlers.c | 14 ++ > 1 file changed, 6 insertions(+), 8 deletions(-) > > diff --git a/multipathd/cli_handlers.c b/multipathd/cli_handlers.c > index 8ff4362..16445ea 100644 > --- a/multipathd/cli_handlers.c > +++ b/multipathd/cli_handlers.c > @@ -1,6 +1,9 @@ > /* > * Copyright (c) 2005 Christophe Varoqui > */ > +#define _GNU_SOURCE > + > +#include > #include "checkers.h" > #include "memory.h" > #include "vector.h" > @@ -1285,14 +1288,9 @@ cli_getprstatus (void * v, char ** reply, int * > len, void * data) > > condlog(3, "%s: prflag = %u", param, (unsigned > int)mpp->prflag); > > - *reply =(char *)malloc(2); > - *len = 2; > - memset(*reply,0,2); > - > - > - sprintf(*reply,"%d",mpp->prflag); > - (*reply)[1]='\0'; > - > + *len = asprintf(reply, "%d\n", mpp->prflag); > + if (*len < 0) > + return 1; > > condlog(3, "%s: reply = %s", param, *reply); Hello Peng, Sorry but returning 1 looks somewhat inconsistent to me. This function is called indirectly by uxsock_trigger() and that function expects that cli_getprstatus() either returns a positive error code (E...) or a negative error code. Please change this patch such that ENOMEM is returned instead of 1 if asprintf() fails. Thanks, Bart. -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] [PATCH] multipathd: fix issue in 'map $map getprstatus' reply
Please have a review for this patch, hope for your comments. Thanks 发件人: peng.lia...@zte.com.cn 收件人: christophe varoqui <christophe.varo...@free.fr>, 抄送: zhang.ka...@zte.com.cn, dm-devel@redhat.com, peng liang <peng.lia...@zte.com.cn> 日期: 2016-08-04 15:31 主题: [dm-devel] [PATCH] multipathd: fix issue in 'map $map getprstatus' reply 发件人: dm-devel-boun...@redhat.com From: peng liang <peng.lia...@zte.com.cn> -add missing newline to 'map|multipath $map getprstatus' reply -use asprintf instead of sprintf Signed-off-by: peng liang <peng.lia...@zte.com.cn> --- multipathd/cli_handlers.c | 14 ++ 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/multipathd/cli_handlers.c b/multipathd/cli_handlers.c index 8ff4362..16445ea 100644 --- a/multipathd/cli_handlers.c +++ b/multipathd/cli_handlers.c @@ -1,6 +1,9 @@ /* * Copyright (c) 2005 Christophe Varoqui */ +#define _GNU_SOURCE + +#include #include "checkers.h" #include "memory.h" #include "vector.h" @@ -1285,14 +1288,9 @@ cli_getprstatus (void * v, char ** reply, int * len, void * data) condlog(3, "%s: prflag = %u", param, (unsigned int)mpp->prflag); -*reply =(char *)malloc(2); -*len = 2; -memset(*reply,0,2); - - -sprintf(*reply,"%d",mpp->prflag); -(*reply)[1]='\0'; - +*len = asprintf(reply, "%d\n", mpp->prflag); +if (*len < 0) +return 1; condlog(3, "%s: reply = %s", param, *reply); -- 2.8.1.windows.1 -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] [PATCH] multipathd: fix issue in 'map $map getprstatus' reply
On 08/03/2016 12:09 AM, peng.lia...@zte.com.cn wrote: + *len = 3; + asprintf(reply, "%d\n", mpp->prflag); Hello Peng, Please use the return value of asprintf() to compute *len instead of assigning a hard-coded constant to *len. Thanks, Bart. -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] [PATCH] multipathd: fix issue in 'map $map getprstatus' reply
Hi Bart, Thanks for your advice! I have updated the solution as follows. diff --git a/multipathd/cli_handlers.c b/multipathd/cli_handlers.c index 0b04504..cec40eb 100644 --- a/multipathd/cli_handlers.c +++ b/multipathd/cli_handlers.c @@ -1,6 +1,9 @@ /* * Copyright (c) 2005 Christophe Varoqui */ +#define _GNU_SOURCE + +#include #include "checkers.h" #include "memory.h" #include "vector.h" @@ -1285,14 +1288,8 @@ cli_getprstatus (void * v, char ** reply, int * len, void * data) condlog(3, "%s: prflag = %u", param, (unsigned int)mpp->prflag); - *reply =(char *)malloc(3); *len = 3; - memset(*reply,0,3); - - - sprintf(*reply,"%d\n",mpp->prflag); - (*reply)[2]='\0'; - + asprintf(reply, "%d\n", mpp->prflag); condlog(3, "%s: reply = %s", param, *reply); 发件人: Bart Van Assche <bart.vanass...@sandisk.com> 收件人: <peng.lia...@zte.com.cn>, christophe varoqui <christophe.varo...@free.fr>, 抄送: <zhang.ka...@zte.com.cn>, <dm-devel@redhat.com> 日期: 2016/08/02 23:20 主题: Re: [dm-devel] [PATCH] multipathd: fix issue in 'map $map getprstatus' reply On 08/01/2016 06:27 PM, peng.lia...@zte.com.cn wrote: > From: peng liang <peng.lia...@zte.com.cn> > > add missing newline to 'map|multipath $map getprstatus' reply > > Signed-off-by: peng liang <peng.lia...@zte.com.cn> > --- > multipathd/cli_handlers.c | 10 +- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/multipathd/cli_handlers.c b/multipathd/cli_handlers.c > index 8ff4362..0b04504 100644 > --- a/multipathd/cli_handlers.c > +++ b/multipathd/cli_handlers.c > @@ -1285,13 +1285,13 @@ cli_getprstatus (void * v, char ** reply, int * len, void * data) > >condlog(3, "%s: prflag = %u", param, (unsigned int)mpp->prflag); > > - *reply =(char *)malloc(2); > - *len = 2; > - memset(*reply,0,2); > + *reply =(char *)malloc(3); > + *len = 3; > + memset(*reply,0,3); > > > - sprintf(*reply,"%d",mpp->prflag); > - (*reply)[1]='\0'; > + sprintf(*reply,"%d\n",mpp->prflag); > + (*reply)[2]='\0'; Hello Peng, Please use asprintf() instead of malloc() + memset() + sprintf(). See also https://www.gnu.org/software/libc/manual/html_node/Dynamic-Output.html Thanks, Bart. -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
[dm-devel] [PATCH] multipathd: fix issue in 'map $map getprstatus' reply
From: peng liang-add missing newline to 'map|multipath $map getprstatus' reply -use asprintf instead of sprintf Signed-off-by: peng liang --- multipathd/cli_handlers.c | 13 + 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/multipathd/cli_handlers.c b/multipathd/cli_handlers.c index 8ff4362..cec40eb 100644 --- a/multipathd/cli_handlers.c +++ b/multipathd/cli_handlers.c @@ -1,6 +1,9 @@ /* * Copyright (c) 2005 Christophe Varoqui */ +#define _GNU_SOURCE + +#include #include "checkers.h" #include "memory.h" #include "vector.h" @@ -1285,14 +1288,8 @@ cli_getprstatus (void * v, char ** reply, int * len, void * data) condlog(3, "%s: prflag = %u", param, (unsigned int)mpp->prflag); - *reply =(char *)malloc(2); - *len = 2; - memset(*reply,0,2); - - - sprintf(*reply,"%d",mpp->prflag); - (*reply)[1]='\0'; - + *len = 3; + asprintf(reply, "%d\n", mpp->prflag); condlog(3, "%s: reply = %s", param, *reply); -- 2.8.1.windows.1 -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] [PATCH] multipathd: fix issue in 'map $map getprstatus' reply
On 08/01/2016 06:27 PM, peng.lia...@zte.com.cn wrote: From: peng liangadd missing newline to 'map|multipath $map getprstatus' reply Signed-off-by: peng liang --- multipathd/cli_handlers.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/multipathd/cli_handlers.c b/multipathd/cli_handlers.c index 8ff4362..0b04504 100644 --- a/multipathd/cli_handlers.c +++ b/multipathd/cli_handlers.c @@ -1285,13 +1285,13 @@ cli_getprstatus (void * v, char ** reply, int * len, void * data) condlog(3, "%s: prflag = %u", param, (unsigned int)mpp->prflag); - *reply =(char *)malloc(2); - *len = 2; - memset(*reply,0,2); + *reply =(char *)malloc(3); + *len = 3; + memset(*reply,0,3); - sprintf(*reply,"%d",mpp->prflag); - (*reply)[1]='\0'; + sprintf(*reply,"%d\n",mpp->prflag); + (*reply)[2]='\0'; Hello Peng, Please use asprintf() instead of malloc() + memset() + sprintf(). See also https://www.gnu.org/software/libc/manual/html_node/Dynamic-Output.html Thanks, Bart. -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel