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

From: Junio C Hamano <[email-masked]>
Date: Tue, 20 Aug 2024 13:36:11 -0700

Patrick Steinhardt <[email-masked]> writes:

> 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 <[email-masked]>
---
 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: