repository: remove duplicate free of cache->squash_msg #2132
+0
−1
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
Contributor
Author
|
/submit |
|
Submitted as [email protected] To fetch this version into To fetch this version to local tag |
Thankfully, it is set to NULL, so no security consequences. However, this is still a mistake that must be rectified. Signed-off-by: Greg Funni <[email protected]>
Contributor
Author
|
/submit |
|
Submitted as [email protected] To fetch this version into To fetch this version to local tag |
|
On the Git mailing list, Eric Sunshine wrote (reply to this): On Thu, Dec 18, 2025 at 10:26 AM AZero13 via GitGitGadget
<[email protected]> wrote:
> Thankfully, it is set to NULL, so no security consequences.
> However, this is still a mistake that must be rectified.
>
> Signed-off-by: Greg Funni <[email protected]>
> ---
> diff --git a/repository.c b/repository.c
> @@ -349,7 +349,6 @@ out:
> static void repo_clear_path_cache(struct repo_path_cache *cache)
> {
> - FREE_AND_NULL(cache->squash_msg);
> FREE_AND_NULL(cache->squash_msg);
> FREE_AND_NULL(cache->merge_msg);
> FREE_AND_NULL(cache->merge_rr);
This mistake has been present since Ævar added this function in
759f340738 (repository.c: free the "path cache" in repo_clear(),
2022-03-04), so it isn't the result of someone else coming along and
adding a new field to the structure which needs freeing but then
botching the call to FREE_AND_NULL(). Moreover, this function does
free all the freeable members of repo_path_cache, hence, nothing is
being leaked, so it must have just been a silly copy/paste mistake in
the first place. Hence, this change makes sense. |
|
User |
|
On the Git mailing list, Junio C Hamano wrote (reply to this): Eric Sunshine <[email protected]> writes:
> On Thu, Dec 18, 2025 at 10:26 AM AZero13 via GitGitGadget
> <[email protected]> wrote:
>> Thankfully, it is set to NULL, so no security consequences.
>> However, this is still a mistake that must be rectified.
>>
>> Signed-off-by: Greg Funni <[email protected]>
>> ---
>> diff --git a/repository.c b/repository.c
>> @@ -349,7 +349,6 @@ out:
>> static void repo_clear_path_cache(struct repo_path_cache *cache)
>> {
>> - FREE_AND_NULL(cache->squash_msg);
>> FREE_AND_NULL(cache->squash_msg);
>> FREE_AND_NULL(cache->merge_msg);
>> FREE_AND_NULL(cache->merge_rr);
>
> This mistake has been present since Ævar added this function in
> 759f340738 (repository.c: free the "path cache" in repo_clear(),
> 2022-03-04), so it isn't the result of someone else coming along and
> adding a new field to the structure which needs freeing but then
> botching the call to FREE_AND_NULL(). Moreover, this function does
> free all the freeable members of repo_path_cache, hence, nothing is
> being leaked, so it must have just been a silly copy/paste mistake in
> the first place. Hence, this change makes sense.
Thanks, both. Will queue.
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Thankfully, it is set to NULL, so no security consequences.
However, this is still a mistake that must be rectified.
Signed-off-by: Greg Funni [email protected]
cc: Eric Sunshine [email protected]