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