[dm-devel] [PATCH 05/14] multipathd: use MALLOC and check return value in, cli_getprkey func

[Benjamin Marzinski]

On Thu, Sep 03, 2020 at 09:13:04PM +0200, Martin Wilck wrote:
> On Wed, 2020-09-02 at 15:18 +0800, lixiaokeng wrote:
> > In cli_getprkey func, we use MALLOC instead of malloc, and check
> > the return value of MALLOC.
> > 
> > Signed-off-by: Zhiqiang Liu <liuzhiqiang26 at huawei.com>
> > Signed-off-by: Lixiaokeng <lixiaokeng at huawei.com>
> > Signed-off-by: Linfeilong <linfeilong at huawei.com>
> > ---
> >  multipathd/cli_handlers.c | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> > 
> > diff --git a/multipathd/cli_handlers.c b/multipathd/cli_handlers.c
> > index 27e4574f..d345afd3 100644
> > --- a/multipathd/cli_handlers.c
> > +++ b/multipathd/cli_handlers.c
> > @@ -1535,7 +1535,11 @@ cli_getprkey(void * v, char ** reply, int *
> > len, void * data)
> >  	if (!mpp)
> >  		return 1;
> > 
> > -	*reply = malloc(26);
> > +	*reply = MALLOC(26);
> > +	if (!*reply) {
> > +		condlog(0, "malloc *reply failed.");
> > +		return 1;
> > +	}
> 
> MALLOC is not necessary (*reply isn't left uninialized), nor is the
> error message.

What's you objection to the error message? Admittedly there is basically
no chance that malloc(26) would ever actually fail. But when things
fail, having error messages so that we can debug them faster is helpful.

If your objection is that malloc checks are mostly just there for good
form, and so those error messages won't actually help in practice, I
agree. But as a general rule, I think we should print error messages on
things that are unambiguoulsy errors.

-Ben

> 
> Regards,
> Martin
>