REFTABLE_REALLOC_ARRAY: fix potential memory leak if realloc failed #1955
+0
−9
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
/submit |
|
Submitted as [email protected] To fetch this version into To fetch this version to local tag |
|
On the Git mailing list, René Scharfe wrote (reply to this): Am 08.05.25 um 15:39 schrieb Lidong Yan via GitGitGadget:
> From: Lidong Yan <[email protected]>
>
> 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é
|
|
User |
|
On the Git mailing list, lidongyan wrote (reply to this): Ok, I will remove it in the next patch.
> 2025年5月9日 05:42，René Scharfe <[email protected]> 写道：
>
> Am 08.05.25 um 15:39 schrieb Lidong Yan via GitGitGadget:
>> From: Lidong Yan <[email protected]>
>>
>> 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é
>
>
|
4d786a0 to
6cc191f
Compare
|
/submit |
|
Submitted as [email protected] To fetch this version into To fetch this version to local tag |
|
On the Git mailing list, Junio C Hamano wrote (reply to this): René Scharfe <[email protected]> writes:
> Am 08.05.25 um 15:39 schrieb Lidong Yan via GitGitGadget:
>> From: Lidong Yan <[email protected]>
>>
>> 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? |
|
On the Git mailing list, Patrick Steinhardt wrote (reply to this): 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 |
|
User |
REFTABLE_REALLOC_ARRAY will cause memory leak if realloc failed. Since it is unused, remove this unsafe macro. Signed-off-by: Lidong Yan <[email protected]>
6cc191f to
107f9ce
Compare
|
On the Git mailing list, lidongyan wrote (reply to this): 2025年5月9日 14:51，Patrick Steinhardt <[email protected]> 写道：
>
> 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. |
|
/submit |
|
Submitted as [email protected] To fetch this version into To fetch this version to local tag |
|
On the Git mailing list, Patrick Steinhardt wrote (reply to this): On Fri, May 09, 2025 at 07:44:46AM +0000, Lidong Yan via GitGitGadget wrote:
> From: Lidong Yan <[email protected]>
>
> 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 |
|
On the Git mailing list, Junio C Hamano wrote (reply to this): Patrick Steinhardt <[email protected]> writes:
> On Fri, May 09, 2025 at 07:44:46AM +0000, Lidong Yan via GitGitGadget wrote:
>> From: Lidong Yan <[email protected]>
>>
>> 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. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
cc: René Scharfe [email protected]
cc: Patrick Steinhardt [email protected]