On Fri, 2020-09-04 at 13:25 -0500, Benjamin Marzinski wrote:
> 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@xxxxxxxxxx>
> > > Signed-off-by: Lixiaokeng <lixiaokeng@xxxxxxxxxx>
> > > Signed-off-by: Linfeilong <linfeilong@xxxxxxxxxx>
> > > ---
> > > 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.
See my reply to 00/14. I'd like to standardize and streamline "out of
memory" error messages, rather than hand-coding them in every
procedure. I think that in 99% of cases, if multipathd crashes or
errors out due to OOM, the information in which function OOM occured
first will not be helpful. Even if we had a major memory leak, its
unlikely that such error messages would help us find it.
Do you disagree?
Martin
--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/dm-devel
