* [PATCH] REFTABLE_REALLOC_ARRAY: fix potential memory leak if realloc failed
@ 2025-05-08 13:39 Lidong Yan via GitGitGadget
2025-05-08 21:42 ` René Scharfe
2025-05-09 2:04 ` [PATCH v2] REFTABLE_REALLOC_ARRAY: remove this unsafe yet unused macro Lidong Yan via GitGitGadget
0 siblings, 2 replies; 10+ messages in thread
From: Lidong Yan via GitGitGadget @ 2025-05-08 13:39 UTC (permalink / raw)
To: git; +Cc: Lidong Yan, Lidong Yan
From: Lidong Yan <502024330056@smail.nju.edu.cn>
REFTABLE_REALLOC_ARRAY doesn't free origin pointer when reftable_realloc
failed. This leak can be fixed by add a free(x) before set x to NULL.
Signed-off-by: Lidong Yan <502024330056@smail.nju.edu.cn>
---
REFTABLE_REALLOC_ARRAY: fix potential memory leak if realloc failed
REFTABLE_REALLOC_ARRAY doesn't free origin pointer when reftable_realloc
failed. This leak can be fixed by add a free(x) before set x to NULL.
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1955%2Fbrandb97%2Ffix-REFTABLE-REALLOC-leak-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1955/brandb97/fix-REFTABLE-REALLOC-leak-v1
Pull-Request: https://github.com/git/git/pull/1955
reftable/basics.h | 28 ++++++++++++++++++++--------
1 file changed, 20 insertions(+), 8 deletions(-)
diff --git a/reftable/basics.h b/reftable/basics.h
index d8888c12629..c7651f0cda8 100644
--- a/reftable/basics.h
+++ b/reftable/basics.h
@@ -200,14 +200,26 @@ static inline int reftable_alloc_size(size_t nelem, size_t elsize, size_t *out)
} \
} while (0)
#define REFTABLE_CALLOC_ARRAY(x, alloc) (x) = reftable_calloc((alloc), sizeof(*(x)))
-#define REFTABLE_REALLOC_ARRAY(x, alloc) do { \
- size_t alloc_size; \
- if (reftable_alloc_size(sizeof(*(x)), (alloc), &alloc_size) < 0) { \
- errno = ENOMEM; \
- (x) = NULL; \
- } else { \
- (x) = reftable_realloc((x), alloc_size); \
- } \
+#define REFTABLE_REALLOC_ARRAY(x, alloc) \
+ do { \
+ size_t alloc_size; \
+ void *new_p; \
+ if (reftable_alloc_size(sizeof(*(x)), (alloc), &alloc_size) < \
+ 0) { \
+ goto cleanup; \
+ } else { \
+ new_p = reftable_realloc((x), alloc_size); \
+ if (!new_p) { \
+ goto cleanup; \
+ } \
+ (x) = new_p; \
+ } \
+ break; \
+ cleanup: \
+ if (x) \
+ free(x); \
+ errno = ENOMEM; \
+ (x) = NULL; \
} while (0)
static inline void *reftable_alloc_grow(void *p, size_t nelem, size_t elsize,
base-commit: 6f84262c44a89851c3ae5a6e4c1a9d06b2068d75
--
gitgitgadget
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] REFTABLE_REALLOC_ARRAY: fix potential memory leak if realloc failed
2025-05-08 13:39 [PATCH] REFTABLE_REALLOC_ARRAY: fix potential memory leak if realloc failed Lidong Yan via GitGitGadget
@ 2025-05-08 21:42 ` René Scharfe
2025-05-09 1:54 ` lidongyan
2025-05-09 3:43 ` Junio C Hamano
2025-05-09 2:04 ` [PATCH v2] REFTABLE_REALLOC_ARRAY: remove this unsafe yet unused macro Lidong Yan via GitGitGadget
1 sibling, 2 replies; 10+ messages in thread
From: René Scharfe @ 2025-05-08 21:42 UTC (permalink / raw)
To: Lidong Yan via GitGitGadget, git; +Cc: Lidong Yan
Am 08.05.25 um 15:39 schrieb Lidong Yan via GitGitGadget:
> From: Lidong Yan <502024330056@smail.nju.edu.cn>
>
> REFTABLE_REALLOC_ARRAY doesn't free origin pointer when reftable_realloc
> failed. This leak can be fixed by add a free(x) before set x to NULL.
Hmm, this macro is unused. Perhaps remove it?
René
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] REFTABLE_REALLOC_ARRAY: fix potential memory leak if realloc failed
2025-05-08 21:42 ` René Scharfe
@ 2025-05-09 1:54 ` lidongyan
2025-05-09 3:43 ` Junio C Hamano
1 sibling, 0 replies; 10+ messages in thread
From: lidongyan @ 2025-05-09 1:54 UTC (permalink / raw)
To: René Scharfe; +Cc: Lidong Yan via GitGitGadget, git
Ok, I will remove it in the next patch.
> 2025年5月9日 05:42，René Scharfe <l.s.r@web.de> 写道：
>
> Am 08.05.25 um 15:39 schrieb Lidong Yan via GitGitGadget:
>> From: Lidong Yan <502024330056@smail.nju.edu.cn>
>>
>> REFTABLE_REALLOC_ARRAY doesn't free origin pointer when reftable_realloc
>> failed. This leak can be fixed by add a free(x) before set x to NULL.
>
> Hmm, this macro is unused. Perhaps remove it?
>
> René
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2] REFTABLE_REALLOC_ARRAY: remove this unsafe yet unused macro
2025-05-08 13:39 [PATCH] REFTABLE_REALLOC_ARRAY: fix potential memory leak if realloc failed Lidong Yan via GitGitGadget
2025-05-08 21:42 ` René Scharfe
@ 2025-05-09 2:04 ` Lidong Yan via GitGitGadget
2025-05-09 6:51 ` Patrick Steinhardt
2025-05-09 7:44 ` [PATCH v3] " Lidong Yan via GitGitGadget
1 sibling, 2 replies; 10+ messages in thread
From: Lidong Yan via GitGitGadget @ 2025-05-09 2:04 UTC (permalink / raw)
To: git; +Cc: René Scharfe, Lidong Yan, Lidong Yan
From: Lidong Yan <502024330056@smail.nju.edu.cn>
REFTABLE_REALLOC_ARRAY will cause memory leak if realloc failed.
Since it is unused, remove this unsafe macro.
Signed-off-by: Lidong Yan <502024330056@smail.nju.edu.cn>
---
REFTABLE_REALLOC_ARRAY: fix potential memory leak if realloc failed
REFTABLE_REALLOC_ARRAY doesn't free origin pointer when reftable_realloc
failed. This leak can be fixed by add a free(x) before set x to NULL.
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1955%2Fbrandb97%2Ffix-REFTABLE-REALLOC-leak-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1955/brandb97/fix-REFTABLE-REALLOC-leak-v2
Pull-Request: https://github.com/git/git/pull/1955
Range-diff vs v1:
1: 4d786a0ec17 ! 1: 6cc191f9db8 REFTABLE_REALLOC_ARRAY: fix potential memory leak if realloc failed
@@ Metadata
Author: Lidong Yan <502024330056@smail.nju.edu.cn>
## Commit message ##
- REFTABLE_REALLOC_ARRAY: fix potential memory leak if realloc failed
+ REFTABLE_REALLOC_ARRAY: remove this unsafe yet unused macro
- REFTABLE_REALLOC_ARRAY doesn't free origin pointer when reftable_realloc
- failed. This leak can be fixed by add a free(x) before set x to NULL.
+ REFTABLE_REALLOC_ARRAY will cause memory leak if realloc failed.
+ Since it is unused, remove this unsafe macro.
Signed-off-by: Lidong Yan <502024330056@smail.nju.edu.cn>
## reftable/basics.h ##
@@ reftable/basics.h: static inline int reftable_alloc_size(size_t nelem, size_t elsize, size_t *out)
+ (x) = reftable_malloc(alloc_size); \
} \
} while (0)
- #define REFTABLE_CALLOC_ARRAY(x, alloc) (x) = reftable_calloc((alloc), sizeof(*(x)))
+-#define REFTABLE_CALLOC_ARRAY(x, alloc) (x) = reftable_calloc((alloc), sizeof(*(x)))
-#define REFTABLE_REALLOC_ARRAY(x, alloc) do { \
- size_t alloc_size; \
- if (reftable_alloc_size(sizeof(*(x)), (alloc), &alloc_size) < 0) { \
@@ reftable/basics.h: static inline int reftable_alloc_size(size_t nelem, size_t el
- } else { \
- (x) = reftable_realloc((x), alloc_size); \
- } \
-+#define REFTABLE_REALLOC_ARRAY(x, alloc) \
-+ do { \
-+ size_t alloc_size; \
-+ void *new_p; \
-+ if (reftable_alloc_size(sizeof(*(x)), (alloc), &alloc_size) < \
-+ 0) { \
-+ goto cleanup; \
-+ } else { \
-+ new_p = reftable_realloc((x), alloc_size); \
-+ if (!new_p) { \
-+ goto cleanup; \
-+ } \
-+ (x) = new_p; \
-+ } \
-+ break; \
-+ cleanup: \
-+ if (x) \
-+ free(x); \
-+ errno = ENOMEM; \
-+ (x) = NULL; \
- } while (0)
+- } while (0)
++#define REFTABLE_CALLOC_ARRAY(x, alloc) \
++ (x) = reftable_calloc((alloc), sizeof(*(x)))
static inline void *reftable_alloc_grow(void *p, size_t nelem, size_t elsize,
+ size_t *allocp)
reftable/basics.h | 12 ++----------
1 file changed, 2 insertions(+), 10 deletions(-)
diff --git a/reftable/basics.h b/reftable/basics.h
index d8888c12629..667feffd935 100644
--- a/reftable/basics.h
+++ b/reftable/basics.h
@@ -199,16 +199,8 @@ static inline int reftable_alloc_size(size_t nelem, size_t elsize, size_t *out)
(x) = reftable_malloc(alloc_size); \
} \
} while (0)
-#define REFTABLE_CALLOC_ARRAY(x, alloc) (x) = reftable_calloc((alloc), sizeof(*(x)))
-#define REFTABLE_REALLOC_ARRAY(x, alloc) do { \
- size_t alloc_size; \
- if (reftable_alloc_size(sizeof(*(x)), (alloc), &alloc_size) < 0) { \
- errno = ENOMEM; \
- (x) = NULL; \
- } else { \
- (x) = reftable_realloc((x), alloc_size); \
- } \
- } while (0)
+#define REFTABLE_CALLOC_ARRAY(x, alloc) \
+ (x) = reftable_calloc((alloc), sizeof(*(x)))
static inline void *reftable_alloc_grow(void *p, size_t nelem, size_t elsize,
size_t *allocp)
base-commit: 6f84262c44a89851c3ae5a6e4c1a9d06b2068d75
--
gitgitgadget
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] REFTABLE_REALLOC_ARRAY: fix potential memory leak if realloc failed
2025-05-08 21:42 ` René Scharfe
2025-05-09 1:54 ` lidongyan
@ 2025-05-09 3:43 ` Junio C Hamano
1 sibling, 0 replies; 10+ messages in thread
From: Junio C Hamano @ 2025-05-09 3:43 UTC (permalink / raw)
To: René Scharfe; +Cc: Lidong Yan via GitGitGadget, git, Lidong Yan
René Scharfe <l.s.r@web.de> writes:
> Am 08.05.25 um 15:39 schrieb Lidong Yan via GitGitGadget:
>> From: Lidong Yan <502024330056@smail.nju.edu.cn>
>>
>> REFTABLE_REALLOC_ARRAY doesn't free origin pointer when reftable_realloc
>> failed. This leak can be fixed by add a free(x) before set x to NULL.
>
> Hmm, this macro is unused. Perhaps remove it?
Or let the sleeping dog alone, perhaps?
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] REFTABLE_REALLOC_ARRAY: remove this unsafe yet unused macro
2025-05-09 2:04 ` [PATCH v2] REFTABLE_REALLOC_ARRAY: remove this unsafe yet unused macro Lidong Yan via GitGitGadget
@ 2025-05-09 6:51 ` Patrick Steinhardt
2025-05-09 7:38 ` lidongyan
2025-05-09 7:44 ` [PATCH v3] " Lidong Yan via GitGitGadget
1 sibling, 1 reply; 10+ messages in thread
From: Patrick Steinhardt @ 2025-05-09 6:51 UTC (permalink / raw)
To: Lidong Yan via GitGitGadget; +Cc: git, René Scharfe, Lidong Yan
On Fri, May 09, 2025 at 02:04:22AM +0000, Lidong Yan via GitGitGadget wrote:
> diff --git a/reftable/basics.h b/reftable/basics.h
> index d8888c12629..667feffd935 100644
> --- a/reftable/basics.h
> +++ b/reftable/basics.h
> @@ -199,16 +199,8 @@ static inline int reftable_alloc_size(size_t nelem, size_t elsize, size_t *out)
> (x) = reftable_malloc(alloc_size); \
> } \
> } while (0)
> -#define REFTABLE_CALLOC_ARRAY(x, alloc) (x) = reftable_calloc((alloc), sizeof(*(x)))
> -#define REFTABLE_REALLOC_ARRAY(x, alloc) do { \
> - size_t alloc_size; \
> - if (reftable_alloc_size(sizeof(*(x)), (alloc), &alloc_size) < 0) { \
> - errno = ENOMEM; \
> - (x) = NULL; \
> - } else { \
> - (x) = reftable_realloc((x), alloc_size); \
> - } \
> - } while (0)
> +#define REFTABLE_CALLOC_ARRAY(x, alloc) \
> + (x) = reftable_calloc((alloc), sizeof(*(x)))
Let's avoid reformatting unrelated macros. But other than that I fully
agree -- we should remove stuff that we don't use in the first place.
Thanks!
Patrick
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] REFTABLE_REALLOC_ARRAY: remove this unsafe yet unused macro
2025-05-09 6:51 ` Patrick Steinhardt
@ 2025-05-09 7:38 ` lidongyan
0 siblings, 0 replies; 10+ messages in thread
From: lidongyan @ 2025-05-09 7:38 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: Lidong Yan via GitGitGadget, git, René Scharfe
2025年5月9日 14:51，Patrick Steinhardt <ps@pks.im> 写道：
>
> On Fri, May 09, 2025 at 02:04:22AM +0000, Lidong Yan via GitGitGadget wrote:
>> diff --git a/reftable/basics.h b/reftable/basics.h
>> index d8888c12629..667feffd935 100644
>> --- a/reftable/basics.h
>> +++ b/reftable/basics.h
>> @@ -199,16 +199,8 @@ static inline int reftable_alloc_size(size_t nelem, size_t elsize, size_t *out)
>> (x) = reftable_malloc(alloc_size); \
>> } \
>> } while (0)
>> -#define REFTABLE_CALLOC_ARRAY(x, alloc) (x) = reftable_calloc((alloc), sizeof(*(x)))
>> -#define REFTABLE_REALLOC_ARRAY(x, alloc) do { \
>> - size_t alloc_size; \
>> - if (reftable_alloc_size(sizeof(*(x)), (alloc), &alloc_size) < 0) { \
>> - errno = ENOMEM; \
>> - (x) = NULL; \
>> - } else { \
>> - (x) = reftable_realloc((x), alloc_size); \
>> - } \
>> - } while (0)
>> +#define REFTABLE_CALLOC_ARRAY(x, alloc) \
>> + (x) = reftable_calloc((alloc), sizeof(*(x)))
>
> Let's avoid reformatting unrelated macros. But other than that I fully
> agree -- we should remove stuff that we don't use in the first place.
>
> Thanks!
>
> Patrick
>
Ok, I will restore REFTABLE_CALLOC_ARRAY in the next patch.
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v3] REFTABLE_REALLOC_ARRAY: remove this unsafe yet unused macro
2025-05-09 2:04 ` [PATCH v2] REFTABLE_REALLOC_ARRAY: remove this unsafe yet unused macro Lidong Yan via GitGitGadget
2025-05-09 6:51 ` Patrick Steinhardt
@ 2025-05-09 7:44 ` Lidong Yan via GitGitGadget
2025-05-09 8:23 ` Patrick Steinhardt
1 sibling, 1 reply; 10+ messages in thread
From: Lidong Yan via GitGitGadget @ 2025-05-09 7:44 UTC (permalink / raw)
To: git; +Cc: René Scharfe, Patrick Steinhardt, Lidong Yan, Lidong Yan
From: Lidong Yan <502024330056@smail.nju.edu.cn>
REFTABLE_REALLOC_ARRAY will cause memory leak if realloc failed.
Since it is unused, remove this unsafe macro.
Signed-off-by: Lidong Yan <502024330056@smail.nju.edu.cn>
---
REFTABLE_REALLOC_ARRAY: fix potential memory leak if realloc failed
REFTABLE_REALLOC_ARRAY doesn't free origin pointer when reftable_realloc
failed. This leak can be fixed by add a free(x) before set x to NULL.
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1955%2Fbrandb97%2Ffix-REFTABLE-REALLOC-leak-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1955/brandb97/fix-REFTABLE-REALLOC-leak-v3
Pull-Request: https://github.com/git/git/pull/1955
Range-diff vs v2:
1: 6cc191f9db8 ! 1: 107f9ce3bd0 REFTABLE_REALLOC_ARRAY: remove this unsafe yet unused macro
@@ Commit message
## reftable/basics.h ##
@@ reftable/basics.h: static inline int reftable_alloc_size(size_t nelem, size_t elsize, size_t *out)
- (x) = reftable_malloc(alloc_size); \
} \
} while (0)
--#define REFTABLE_CALLOC_ARRAY(x, alloc) (x) = reftable_calloc((alloc), sizeof(*(x)))
+ #define REFTABLE_CALLOC_ARRAY(x, alloc) (x) = reftable_calloc((alloc), sizeof(*(x)))
-#define REFTABLE_REALLOC_ARRAY(x, alloc) do { \
- size_t alloc_size; \
- if (reftable_alloc_size(sizeof(*(x)), (alloc), &alloc_size) < 0) { \
@@ reftable/basics.h: static inline int reftable_alloc_size(size_t nelem, size_t el
- (x) = reftable_realloc((x), alloc_size); \
- } \
- } while (0)
-+#define REFTABLE_CALLOC_ARRAY(x, alloc) \
-+ (x) = reftable_calloc((alloc), sizeof(*(x)))
static inline void *reftable_alloc_grow(void *p, size_t nelem, size_t elsize,
size_t *allocp)
reftable/basics.h | 9 ---------
1 file changed, 9 deletions(-)
diff --git a/reftable/basics.h b/reftable/basics.h
index d8888c12629..1674a6f8b80 100644
--- a/reftable/basics.h
+++ b/reftable/basics.h
@@ -200,15 +200,6 @@ static inline int reftable_alloc_size(size_t nelem, size_t elsize, size_t *out)
} \
} while (0)
#define REFTABLE_CALLOC_ARRAY(x, alloc) (x) = reftable_calloc((alloc), sizeof(*(x)))
-#define REFTABLE_REALLOC_ARRAY(x, alloc) do { \
- size_t alloc_size; \
- if (reftable_alloc_size(sizeof(*(x)), (alloc), &alloc_size) < 0) { \
- errno = ENOMEM; \
- (x) = NULL; \
- } else { \
- (x) = reftable_realloc((x), alloc_size); \
- } \
- } while (0)
static inline void *reftable_alloc_grow(void *p, size_t nelem, size_t elsize,
size_t *allocp)
base-commit: 6f84262c44a89851c3ae5a6e4c1a9d06b2068d75
--
gitgitgadget
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v3] REFTABLE_REALLOC_ARRAY: remove this unsafe yet unused macro
2025-05-09 7:44 ` [PATCH v3] " Lidong Yan via GitGitGadget
@ 2025-05-09 8:23 ` Patrick Steinhardt
2025-05-09 14:25 ` Junio C Hamano
0 siblings, 1 reply; 10+ messages in thread
From: Patrick Steinhardt @ 2025-05-09 8:23 UTC (permalink / raw)
To: Lidong Yan via GitGitGadget; +Cc: git, René Scharfe, Lidong Yan
On Fri, May 09, 2025 at 07:44:46AM +0000, Lidong Yan via GitGitGadget wrote:
> From: Lidong Yan <502024330056@smail.nju.edu.cn>
>
> REFTABLE_REALLOC_ARRAY will cause memory leak if realloc failed.
> Since it is unused, remove this unsafe macro.
This looks good to me, thanks!
Patrick
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3] REFTABLE_REALLOC_ARRAY: remove this unsafe yet unused macro
2025-05-09 8:23 ` Patrick Steinhardt
@ 2025-05-09 14:25 ` Junio C Hamano
0 siblings, 0 replies; 10+ messages in thread
From: Junio C Hamano @ 2025-05-09 14:25 UTC (permalink / raw)
To: Patrick Steinhardt
Cc: Lidong Yan via GitGitGadget, git, René Scharfe, Lidong Yan
Patrick Steinhardt <ps@pks.im> writes:
> On Fri, May 09, 2025 at 07:44:46AM +0000, Lidong Yan via GitGitGadget wrote:
>> From: Lidong Yan <502024330056@smail.nju.edu.cn>
>>
>> REFTABLE_REALLOC_ARRAY will cause memory leak if realloc failed.
>> Since it is unused, remove this unsafe macro.
>
> This looks good to me, thanks!
Alright. As long as we do not foresee adding a new caller of this
in the near future, removal is always preferred ;-)
Thanks.
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2025-05-09 14:26 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-05-08 13:39 [PATCH] REFTABLE_REALLOC_ARRAY: fix potential memory leak if realloc failed Lidong Yan via GitGitGadget
2025-05-08 21:42 ` René Scharfe
2025-05-09 1:54 ` lidongyan
2025-05-09 3:43 ` Junio C Hamano
2025-05-09 2:04 ` [PATCH v2] REFTABLE_REALLOC_ARRAY: remove this unsafe yet unused macro Lidong Yan via GitGitGadget
2025-05-09 6:51 ` Patrick Steinhardt
2025-05-09 7:38 ` lidongyan
2025-05-09 7:44 ` [PATCH v3] " Lidong Yan via GitGitGadget
2025-05-09 8:23 ` Patrick Steinhardt
2025-05-09 14:25 ` Junio C Hamano
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).