git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 03/20] pretty: fix memory leaks when parsing pretty formats

From
Junio C Hamano <gitster@pobox.com>
Date
Aug 20, 2024, 20:36 UTC
Message-ID
<xmqq7ccb6if8.fsf@gitster.g>
In-Reply-To
<82f3908f9620cee29e36a51f6d18ddcc8392b966.1724159575.git.ps@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
Show 10 quoted lines
> When parsing pretty formats from the config we leak the name and user
> format whenever these are set multiple times. This is because we do not
> free any already-set value in case there is one.
>
> Plugging this leak for the name is trivial. For the user format we need
> to be a bit more careful, because we may end up assigning a pointer into
> the allocated region when the string is prefixed with either "format" or
> "tformat:". In order to make it safe to unconditionally free the user
> format we thus strdup the stripped string into the field instead of a
> pointer into the string.
Yup, the stripped one is trickier, but the change looks correct.
Will queue.

By the way, we tend to prefer no spaces after (cast) in our codebase, but I just noticed that it is not spelled out in the coding guidelines. Comparing

    $ git grep -E -e '\((int|char \*)\) ' \*.c ':!compat/' ':!contrib/'
    $ git grep -E -e '\((int|char \*)\)[^ ]' \*.c ':!compat/' ':!contrib/'

tells me that the extra space after the (cast) is found mostly in borrowed or imported sources and majority of culprits are found in reftable library X-<.

Thanks.
--- >8 ---
Subject: CodingGuidelines: spaces around C operators

As we have operated with "write like how your surrounding code is written" for too long, after a huge code drop from another project, we'll end up being inconsistent before such an imported code is cleaned up. We have many uses of cast operator with a space before its operand, mostly in the reftable code.

Spell the convention out before it spreads to other places.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/CodingGuidelines | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)
diff --git c/Documentation/CodingGuidelines w/Documentation/CodingGuidelines
index e4bd0abdcd..ccaea39752 100644
--- c/Documentation/CodingGuidelines
+++ w/Documentation/CodingGuidelines
@@ -303,7 +303,9 @@ For C programs:
      v12.01, 2022-03-28).
 
  - Variables have to be declared at the beginning of the block, before
-   the first statement (i.e. -Wdeclaration-after-statement).
+   the first statement (i.e. -Wdeclaration-after-statement).  It is
+   encouraged to have a blank line between the end of the declarations
+   and the first statement in the block.
 
  - NULL pointers shall be written as NULL, not as 0.
 
@@ -323,6 +325,13 @@ For C programs:
         while( condition )
 		func (bar+1);
 
+ - A binary operator (other than ",") and ternary conditional "?:"
+   have a space on each side of the operator to separate it from its
+   operands.  E.g. "A + 1", not "A+1".
+
+ - A unary operator (other than "." and "->") have no space between it
+   and its operand.  E.g. "(char *)ptr", not "(char *) ptr".
+
  - Do not explicitly compare an integral value with constant 0 or '\0',
    or a pointer value with constant NULL.  For instance, to validate that
    counted array <ptr, cnt> is initialized but has no elements, write:
Previous: Patrick SteinhardtNext: Patrick Steinhardt
Message 7 of 60 in “Memory leak fixes (pt.5)”
  1. 00/20 Memory leak fixes (pt.5)Patrick Steinhardt, Aug 20, 2024
  2. 01/20 mailinfo: fix leaking header dataPatrick Steinhardt, Aug 20, 2024
  3. Junio C HamanoAug 20, 2024
  4. 02/20 convert: fix leaks when resetting attributesPatrick Steinhardt, Aug 20, 2024
  5. Junio C HamanoAug 20, 2024
  6. 03/20 pretty: fix memory leaks when parsing pretty formatsPatrick Steinhardt, Aug 20, 2024
  7. Junio C HamanoAug 20, 2024
  8. Patrick SteinhardtAug 22, 2024
  9. 04/20 pretty: fix leaking key/value separator bufferPatrick Steinhardt, Aug 20, 2024
  10. 05/20 builtin/merge-tree: fix leaking `-X` strategy optionsPatrick Steinhardt, Aug 20, 2024
  11. Junio C HamanoAug 20, 2024
  12. 06/20 builtin/upload-archive: fix leaking args passed to `write_archive()`Patrick Steinhardt, Aug 20, 2024
  13. 07/20 builtin/archive: fix leaking `OPT_FILENAME()` valuePatrick Steinhardt, Aug 20, 2024
  14. 08/20 midx-write: fix leaking hashfile on error casesPatrick Steinhardt, Aug 20, 2024
  15. Junio C HamanoAug 20, 2024
  16. Patrick SteinhardtAug 22, 2024
  17. 09/20 builtin/repack: fix leaks when computing packs to repackPatrick Steinhardt, Aug 20, 2024
  18. 10/20 t/helper: fix leaking multi-pack-indices in "read-midx"Patrick Steinhardt, Aug 20, 2024
  19. 11/20 transport: fix leaking OID arrays in git:// transport dataPatrick Steinhardt, Aug 20, 2024
  20. 12/20 builtin/send-pack: fix leaking refspecsPatrick Steinhardt, Aug 20, 2024
  21. 13/20 sideband: fix leaks when configuring sideband colorsPatrick Steinhardt, Aug 20, 2024
  22. Junio C HamanoAug 20, 2024
  23. Patrick SteinhardtAug 22, 2024
  24. 14/20 builtin/fetch-pack: fix leaking refsPatrick Steinhardt, Aug 20, 2024
  25. Junio C HamanoAug 21, 2024
  26. 15/20 remote: fix leaking config stringsPatrick Steinhardt, Aug 20, 2024
  27. Junio C HamanoAug 21, 2024
  28. Patrick SteinhardtAug 22, 2024
  29. Junio C HamanoAug 22, 2024
  30. 16/20 remote: fix leaks when matching refspecsPatrick Steinhardt, Aug 20, 2024
  31. 17/20 remote: fix leaking peer ref when expanding refmapPatrick Steinhardt, Aug 20, 2024
  32. 18/20 builtin/fetch: fix leaking transaction with `--atomic`Patrick Steinhardt, Aug 20, 2024
  33. 19/20 transport: fix leaking arguments when fetching from bundlePatrick Steinhardt, Aug 20, 2024
  34. Junio C HamanoAug 21, 2024
  35. Patrick SteinhardtAug 22, 2024
  36. Junio C HamanoAug 22, 2024
  37. 20/20 transport: fix leaking negotiation tipsPatrick Steinhardt, Aug 20, 2024
  38. 00/20 Memory leak fixes (pt.5)Patrick Steinhardt, Aug 22, 2024
  39. 01/20 mailinfo: fix leaking header dataPatrick Steinhardt, Aug 22, 2024
  40. 02/20 convert: fix leaks when resetting attributesPatrick Steinhardt, Aug 22, 2024
  41. 03/20 pretty: fix memory leaks when parsing pretty formatsPatrick Steinhardt, Aug 22, 2024
  42. 04/20 pretty: fix leaking key/value separator bufferPatrick Steinhardt, Aug 22, 2024
  43. 05/20 builtin/merge-tree: fix leaking `-X` strategy optionsPatrick Steinhardt, Aug 22, 2024
  44. 06/20 builtin/upload-archive: fix leaking args passed to `write_archive()`Patrick Steinhardt, Aug 22, 2024
  45. 07/20 builtin/archive: fix leaking `OPT_FILENAME()` valuePatrick Steinhardt, Aug 22, 2024
  46. 08/20 midx-write: fix leaking hashfile on error casesPatrick Steinhardt, Aug 22, 2024
  47. 09/20 builtin/repack: fix leaks when computing packs to repackPatrick Steinhardt, Aug 22, 2024
  48. 10/20 t/helper: fix leaking multi-pack-indices in "read-midx"Patrick Steinhardt, Aug 22, 2024
  49. 11/20 transport: fix leaking OID arrays in git:// transport dataPatrick Steinhardt, Aug 22, 2024
  50. 12/20 builtin/send-pack: fix leaking refspecsPatrick Steinhardt, Aug 22, 2024
  51. 13/20 sideband: fix leaks when configuring sideband colorsPatrick Steinhardt, Aug 22, 2024
  52. 14/20 builtin/fetch-pack: fix leaking refsPatrick Steinhardt, Aug 22, 2024
  53. 15/20 remote: fix leaking config stringsPatrick Steinhardt, Aug 22, 2024
  54. 16/20 remote: fix leaks when matching refspecsPatrick Steinhardt, Aug 22, 2024
  55. 17/20 remote: fix leaking peer ref when expanding refmapPatrick Steinhardt, Aug 22, 2024
  56. 18/20 builtin/fetch: fix leaking transaction with `--atomic`Patrick Steinhardt, Aug 22, 2024
  57. 19/20 transport: fix leaking arguments when fetching from bundlePatrick Steinhardt, Aug 22, 2024
  58. Junio C HamanoAug 22, 2024
  59. 20/20 transport: fix leaking negotiation tipsPatrick Steinhardt, Aug 22, 2024
  60. Junio C HamanoAug 22, 2024

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.