From: Andrzej Hunt <andrzej@ahunt.org>
To: "René Scharfe" <l.s.r@web.de>,
"Andrzej Hunt via GitGitGadget" <gitgitgadget@gmail.com>,
git@vger.kernel.org
Cc: Andrzej Hunt <ajrhunt@google.com>
Subject: Re: [PATCH 03/12] ls-files: free max_prefix when done
Date: Sun, 25 Apr 2021 15:16:34 +0200 [thread overview]
Message-ID: <11ae3ce9-997b-32fc-7bc3-ee95a3d99153@ahunt.org> (raw)
In-Reply-To: <6a72a920-134f-541b-7caa-debe24658005@web.de>
On 10/04/2021 10:12, René Scharfe wrote:
> Am 09.04.21 um 20:47 schrieb Andrzej Hunt via GitGitGadget:
>> From: Andrzej Hunt <ajrhunt@google.com>
>> diff --git a/builtin/ls-files.c b/builtin/ls-files.c
>> index 60a2913a01e9..53e20bbf9cce 100644
>> --- a/builtin/ls-files.c
>> +++ b/builtin/ls-files.c
>> @@ -781,5 +781,6 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)
>> }
>>
>> dir_clear(&dir);
>> + free((void *)max_prefix);
>
> This cast is necessary to ignore the const attribute of the pointer.
> It's scary, but safe here because this function owns the referenced
> object.
>
> I think the promise to not modify the string given at the top of the
> function is not worth having to take back that promise forcefully at
> the end to dispose of it. Determining the correctness of this cast
> requires reading the whole function. Removing the const from the
> declaration (and the cast) would improve readability overall. Thoughts?
I agree - I'll change this in V2 V2. In fact, Peff already given the
following explanation for why non-const is preferred in this scenario on
a previous patch of mine (which I failed to heed when preparing this patch):
> If a variable is meant to take ownership of memory, our usual
> convention is to not declare it as "const"."
https://lore.kernel.org/git/YEZ0jLppB9wOg%2Faf@coredump.intra.peff.net/
>
>> return 0;
>> }
>>
>
next prev parent reply other threads:[~2021-04-25 13:16 UTC|newest]
Thread overview: 35+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-04-09 18:47 [PATCH 00/12] Fix all leaks in tests t0002-t0099: Part 1 Andrzej Hunt via GitGitGadget
2021-04-09 18:47 ` [PATCH 01/12] revision: free remainder of old commit list in limit_list Andrzej Hunt via GitGitGadget
2021-04-10 7:29 ` René Scharfe
2021-04-25 13:32 ` Andrzej Hunt
2021-04-09 18:47 ` [PATCH 02/12] wt-status: fix multiple small leaks Andrzej Hunt via GitGitGadget
2021-04-09 18:47 ` [PATCH 03/12] ls-files: free max_prefix when done Andrzej Hunt via GitGitGadget
2021-04-10 8:12 ` René Scharfe
2021-04-25 13:16 ` Andrzej Hunt [this message]
2021-04-09 18:47 ` [PATCH 04/12] bloom: clear each bloom_key after use Andrzej Hunt via GitGitGadget
2021-04-11 7:26 ` SZEDER Gábor
2021-04-25 13:17 ` Andrzej Hunt
2021-04-09 18:47 ` [PATCH 05/12] branch: FREE_AND_NULL instead of NULL'ing real_ref Andrzej Hunt via GitGitGadget
2021-04-09 18:47 ` [PATCH 06/12] builtin/bugreport: don't leak prefixed filename Andrzej Hunt via GitGitGadget
2021-04-09 18:47 ` [PATCH 07/12] builtin/check-ignore: clear_pathspec before returning Andrzej Hunt via GitGitGadget
2021-04-09 18:47 ` [PATCH 08/12] builtin/checkout: clear pending objects after diffing Andrzej Hunt via GitGitGadget
2021-04-09 18:47 ` [PATCH 09/12] mailinfo: also free strbuf lists when clearing mailinfo Andrzej Hunt via GitGitGadget
2021-04-11 11:43 ` Junio C Hamano
2021-04-25 13:15 ` Andrzej Hunt
2021-04-09 18:47 ` [PATCH 10/12] builtin/for-each-ref: free filter and UNLEAK sorting Andrzej Hunt via GitGitGadget
2021-04-09 18:47 ` [PATCH 11/12] builtin/rebase: release git_format_patch_opt too Andrzej Hunt via GitGitGadget
2021-04-09 18:47 ` [PATCH 12/12] builtin/rm: avoid leaking pathspec and seen Andrzej Hunt via GitGitGadget
2021-04-25 14:16 ` [PATCH v2 00/12] Fix all leaks in tests t0002-t0099: Part 1 Andrzej Hunt via GitGitGadget
2021-04-25 14:16 ` [PATCH v2 01/12] revision: free remainder of old commit list in limit_list Andrzej Hunt via GitGitGadget
2021-04-25 14:16 ` [PATCH v2 02/12] wt-status: fix multiple small leaks Andrzej Hunt via GitGitGadget
2021-04-25 14:16 ` [PATCH v2 03/12] ls-files: free max_prefix when done Andrzej Hunt via GitGitGadget
2021-04-25 14:16 ` [PATCH v2 04/12] bloom: clear each bloom_key after use Andrzej Hunt via GitGitGadget
2021-04-25 14:16 ` [PATCH v2 05/12] branch: FREE_AND_NULL instead of NULL'ing real_ref Andrzej Hunt via GitGitGadget
2021-04-25 14:16 ` [PATCH v2 06/12] builtin/bugreport: don't leak prefixed filename Andrzej Hunt via GitGitGadget
2021-04-25 14:16 ` [PATCH v2 07/12] builtin/check-ignore: clear_pathspec before returning Andrzej Hunt via GitGitGadget
2021-04-25 14:16 ` [PATCH v2 08/12] builtin/checkout: clear pending objects after diffing Andrzej Hunt via GitGitGadget
2021-04-25 14:16 ` [PATCH v2 09/12] mailinfo: also free strbuf lists when clearing mailinfo Andrzej Hunt via GitGitGadget
2021-04-28 0:43 ` Junio C Hamano
2021-04-25 14:16 ` [PATCH v2 10/12] builtin/for-each-ref: free filter and UNLEAK sorting Andrzej Hunt via GitGitGadget
2021-04-25 14:16 ` [PATCH v2 11/12] builtin/rebase: release git_format_patch_opt too Andrzej Hunt via GitGitGadget
2021-04-25 14:16 ` [PATCH v2 12/12] builtin/rm: avoid leaking pathspec and seen Andrzej Hunt via GitGitGadget
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=11ae3ce9-997b-32fc-7bc3-ee95a3d99153@ahunt.org \
--to=andrzej@ahunt.org \
--cc=ajrhunt@google.com \
--cc=git@vger.kernel.org \
--cc=gitgitgadget@gmail.com \
--cc=l.s.r@web.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
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 read-only IMAP folder(s) and NNTP newsgroup(s).