On 10/04/2021 10:12, René Scharfe wrote:

Am 09.04.21 um 20:47 schrieb Andrzej Hunt via GitGitGadget:

quoted

From: Andrzej Hunt <redacted>

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/ (local)

quoted

  	return 0;
  }