All of lore.kernel.org
 help / color / mirror / Atom feed
From: Benjamin Marzinski <bmarzins@redhat.com>
To: Martin Wilck <mwilck@suse.com>
Cc: lixiaokeng <lixiaokeng@huawei.com>,
	dm-devel mailing list <dm-devel@redhat.com>,
	linfeilong <linfeilong@huawei.com>,
	"liuzhiqiang (I)" <liuzhiqiang26@huawei.com>
Subject: Re: [PATCH 00/14] multipath-tools series: some cleanups and fixes checked by codedex tool
Date: Fri, 4 Sep 2020 15:00:32 -0500	[thread overview]
Message-ID: <20200904200032.GB11108@octiron.msp.redhat.com> (raw)
In-Reply-To: <73d14f51e1ca6aeb8dbd40f5a22c21508b6d4401.camel@suse.com>

On Thu, Sep 03, 2020 at 10:08:53PM +0200, Martin Wilck wrote:
> Hello Lixiaokeng,
> 
> On Wed, 2020-09-02 at 14:40 +0800, lixiaokeng wrote:
> > Hi:
> >     Now, we check multipath-tools codes with codedex tool. Here
> > are some some cleanups and fixes.
> 
> Thank you. However I'm going to nack all patches that add error
> messages after unsuccesful memory allocations. Such messages are
> unhelpful most of the time, and increase the code size without a true
> benefit. I've actually considered to get rid of all these, and replace
> them by a log_oom() macro.

O.k. This answers my question from patch 0005. I'm fine with this.

As a side note: man, those are some ugly preprocessor hoops you need to
jump through to stringify __LINE__.

-Ben

> 
> See an untested prototype attached, to better understand what I mean.
> 
> Regards
> Martin
> 
> 

> From fbbca2c5076a489ee4ae643d6d9199ca5085be95 Mon Sep 17 00:00:00 2001
> From: Martin Wilck <mwilck@suse.com>
> Date: Thu, 3 Sep 2020 22:03:22 +0200
> Subject: [PATCH] libmultipath: prototype implementation of log_oom()
> 
> Rationale: with VERBOSE_OOM_LOGGING, we log the part of the code
> where OOM occured, with minimal runtime effort (no string formatting).
> With lots of log_oom() invocations, our binary will increase by many
> static strings. Without VERBOSE_OOM_LOGGING, we just print a minimal
> error message, which will be enough most of the time.
> 
> Signed-off-by: Martin Wilck <mwilck@suse.com>
> ---
>  libmultipath/debug.c | 7 +++++++
>  libmultipath/debug.h | 8 ++++++++
>  2 files changed, 15 insertions(+)
> 
> diff --git a/libmultipath/debug.c b/libmultipath/debug.c
> index 4128cb9..9062581 100644
> --- a/libmultipath/debug.c
> +++ b/libmultipath/debug.c
> @@ -48,3 +48,10 @@ void dlog (int sink, int prio, const char * fmt, ...)
>  	}
>  	va_end(ap);
>  }
> +
> +#ifndef VERBOSE_OOM_LOGGING
> +void log_oom(void)
> +{
> +	condlog(0, "Out of memory");
> +}
> +#endif
> diff --git a/libmultipath/debug.h b/libmultipath/debug.h
> index c6120c1..f61ecb6 100644
> --- a/libmultipath/debug.h
> +++ b/libmultipath/debug.h
> @@ -11,3 +11,11 @@ extern int logsink;
>  
>  #define condlog(prio, fmt, args...) \
>  	dlog(logsink, prio, fmt "\n", ##args)
> +
> +#ifdef VERBOSE_OOM_LOGGING
> +#define __log_oom(file, line) condlog(0, "Out of memory in " file ":" #line)
> +#define _log_oom(file, line) __log_oom(file, line)
> +#define log_oom() _log_oom(__FILE__, __LINE__)
> +#else
> +void log_oom(void);
> +#endif
> -- 
> 2.28.0
> 

  parent reply	other threads:[~2020-09-04 20:00 UTC|newest]

Thread overview: 47+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-02  6:40 [PATCH 00/14] multipath-tools series: some cleanups and fixes checked by codedex tool lixiaokeng
2020-09-02  7:15 ` [PATCH 01/14] multipathd: return if dm_get_major_minor failed in, cli_add_map lixiaokeng
2020-09-03 17:26   ` Martin Wilck
2020-09-04  2:48     ` lixiaokeng
2020-09-04 13:13       ` Martin Wilck
2020-09-02  7:16 ` [PATCH 02/14] libmultipath: check malloc return value in, print_foreign_topology lixiaokeng
2020-09-03 17:29   ` Martin Wilck
2020-09-02  7:17 ` [PATCH 03/14] libmultipath: use map instead of dm_task_get_name lixiaokeng
2020-09-03 17:35   ` Martin Wilck
2020-09-02  7:17 ` [PATCH 04/14] multipathd: check MALLOC return value in, mpath_pr_event_handler_fn lixiaokeng
2020-09-03 18:57   ` Martin Wilck
2020-09-02  7:18 ` [PATCH 05/14] multipathd: use MALLOC and check return value in, cli_getprkey func lixiaokeng
2020-09-03 19:13   ` Martin Wilck
2020-09-04 18:25     ` Benjamin Marzinski
2020-09-04 21:24       ` Martin Wilck
2020-09-02  7:19 ` [PATCH 06/14] kpartx: check return value of malloc in main func lixiaokeng
2020-09-03 19:23   ` Martin Wilck
2020-09-02  7:19 ` [PATCH 07/14] libmultipath: check return value of dm_mapname in, sysfs_check_holders lixiaokeng
2020-09-04 20:28   ` Benjamin Marzinski
2020-09-02  7:20 ` [PATCH 08/14] libmultipath: donot free *dst if REALLOC fails in, merge_words lixiaokeng
2020-09-04 21:11   ` Benjamin Marzinski
2020-09-07 11:58     ` lixiaokeng
2020-09-02  7:21 ` [PATCH 09/14] libmultipath: check whether mp->features is NULl in, assemble_map lixiaokeng
2020-09-04 21:30   ` Benjamin Marzinski
2020-09-07 12:21     ` lixiaokeng
2020-09-08 15:45       ` Benjamin Marzinski
2020-09-08 16:35         ` Martin Wilck
2020-09-08 16:44           ` Benjamin Marzinski
2020-09-09  3:18           ` lixiaokeng
2020-09-09 10:11             ` Martin Wilck
2020-09-02  7:22 ` [PATCH 10/14] util/tests: use assert_non_null to ensure malloc, returns non-null pointer lixiaokeng
2020-09-04 21:31   ` Benjamin Marzinski
2020-09-02  7:23 ` [PATCH 11/14] mpathpersist: check whether malloc paramp->trnptid_list, fails in handle_args func lixiaokeng
2020-09-04 23:52   ` Benjamin Marzinski
2020-09-07 12:26     ` lixiaokeng
2020-09-02  7:24 ` [PATCH 12/14] libmultipathpersist: use update_multipath_table/status, in get_mpvec lixiaokeng
2020-09-05  0:05   ` Benjamin Marzinski
2020-09-07 13:26     ` lixiaokeng
2020-09-07 14:33     ` Martin Wilck
2020-09-02  7:25 ` [PATCH 13/14] multipath: use update_multipath_table/status in, check_useable_paths lixiaokeng
2020-09-05  0:10   ` Benjamin Marzinski
2020-09-02  7:26 ` [PATCH 14/14] multipathpersist: delete unused variable in handle_args lixiaokeng
2020-09-05  0:14   ` Benjamin Marzinski
2020-09-03 20:08 ` [PATCH 00/14] multipath-tools series: some cleanups and fixes checked by codedex tool Martin Wilck
2020-09-04  2:24   ` lixiaokeng
2020-09-04 20:00   ` Benjamin Marzinski [this message]
2020-09-04 21:28     ` Martin Wilck

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200904200032.GB11108@octiron.msp.redhat.com \
    --to=bmarzins@redhat.com \
    --cc=dm-devel@redhat.com \
    --cc=linfeilong@huawei.com \
    --cc=liuzhiqiang26@huawei.com \
    --cc=lixiaokeng@huawei.com \
    --cc=mwilck@suse.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.