<font size=2 face="@Arial Unicode MS">Hi, Johannes,</font>
<br><font size=2 face="@Arial Unicode MS">Your advice is nice</font><font size=3 face="@Arial Unicode MS">,
thank you</font>
<br><font size=2 face="@Arial Unicode MS">I will send the modified patch
later.</font><font size=2 face="sans-serif"><br>
</font>
<br>
<br>
<br>
<br><font size=1 color=#5f5f5f face="sans-serif">发件人:
</font><font size=1 face="sans-serif">Johannes Thumshirn
<jthumshirn@suse.de></font>
<br><font size=1 color=#5f5f5f face="sans-serif">收件人:
</font><font size=1 face="sans-serif">tang.junhui@zte.com.cn,
</font>
<br><font size=1 color=#5f5f5f face="sans-serif">抄送:
</font><font size=1 face="sans-serif">christophe varoqui
<christophe.varoqui@free.fr>, zhang.kai16@zte.com.cn, dm-devel@redhat.com</font>
<br><font size=1 color=#5f5f5f face="sans-serif">日期:
</font><font size=1 face="sans-serif">2016/08/15
21:59</font>
<br><font size=1 color=#5f5f5f face="sans-serif">主题:
</font><font size=1 face="sans-serif">Re: [dm-devel]
[PATCH] libmultipath: fix memory API logic error</font>
<br>
<hr noshade>
<br>
<br>
<br><tt><font size=2>On Mon, Aug 15, 2016 at 06:59:09PM +0800, tang.junhui@zte.com.cn
wrote:<br>
> From: "tang.junhui" <tang.junhui@zte.com.cn><br>
> <br>
> Memroy API use mem_allocated to record the total size of used memory,<br>
> however, it's wrong to use size(p) as the length of freed memory in
xfree(),<br>
> and memory may also be allocated by STRDUP() or REALLOC(), which is<br>
> not calculated into mem_allocated, so the total size of used memory
is<br>
> not correctly. For these reasons, we removed these incorrectly code
to keep<br>
> the code clean.<br>
> <br>
> Signed-off-by: tang.junhui <tang.junhui@zte.com.cn><br>
> ---<br>
> libmpathpersist/mpath_updatepr.c | 1 -<br>
> libmultipath/memory.c |
31 -------------------------------<br>
> libmultipath/memory.h |
13 ++-----------<br>
> 3 files changed, 2 insertions(+), 43 deletions(-)<br>
> <br>
> diff --git a/libmpathpersist/mpath_updatepr.c b/libmpathpersist/mpath_updatepr.c<br>
> index 9ff4b30..5af2e03 100644<br>
> --- a/libmpathpersist/mpath_updatepr.c<br>
> +++ b/libmpathpersist/mpath_updatepr.c<br>
> @@ -16,7 +16,6 @@<br>
> #include "uxsock.h"<br>
> #include "memory.h"<br>
> <br>
> -unsigned long mem_allocated; /* Total memory used in
Bytes */<br>
> <br>
> int update_prflag(char * arg1, char * arg2, int noisy)<br>
> {<br>
> diff --git a/libmultipath/memory.c b/libmultipath/memory.c<br>
> index 1366f45..5441e6a 100644<br>
> --- a/libmultipath/memory.c<br>
> +++ b/libmultipath/memory.c<br>
> @@ -22,37 +22,6 @@<br>
><br>
<br>
[...]<br>
<br>
> -<br>
> -void<br>
> -xfree(void *p)<br>
> -{<br>
> -
mem_allocated -= sizeof (p);<br>
> -
free(p);<br>
> -
p = NULL;<br>
> -}<br>
> -<br>
> /*<br>
<br>
[...]<br>
<br>
> -#define MALLOC(n) (zalloc(n))<br>
> -#define FREE(p) (xfree(p))<br>
> +#define MALLOC(n) (calloc(1,(n)))<br>
> +#define FREE(p) (free(p))<br>
> #define REALLOC(p,n) (realloc((p),(n)))<br>
> #define STRDUP(n) (strdup(n))<br>
<br>
One minor nit, the original xfree() has set the freed pointer to 0, so
maybe a<br>
<br>
#define FREE(p) do { free(p); p = NULL } while(0)<br>
<br>
would be advisable. I personally like setting free'd pointers to NULL,<br>
as a NULL pointer access is a more immediate feedback than some rather<br>
nasty use-after-free.<br>
<br>
Just my $0.2<br>
<br>
Byte,<br>
Johannes<br>
<br>
-- <br>
Johannes Thumshirn
Storage<br>
jthumshirn@suse.de
+49 911 74053 689<br>
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg<br>
GF: Felix Imendörffer, Jane Smithard, Graham Norton<br>
HRB 21284 (AG Nürnberg)<br>
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850<br>
</font></tt>
<br>