Re: [PATCH 03/12] ls-files: free max_prefix when done

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





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@xxxxxxxxxx>
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@xxxxxxxxxxxxxxxxxxxxxxx/


  	return 0;
  }





[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux