Re: Re* [PATCH v4] submodule: port subcommand 'set-url' from shell to C

From: Eric Sunshine <sunshine@sunshineco.com>
Date: May 8, 2020, 16:13 UTC
Message-ID: <20200508161315.GA3504@flurp.local>
In-Reply-To: <xmqqpnbezaga.fsf@gitster.c.googlers.com>

On Fri, May 08, 2020 at 08:57:09AM -0700, Junio C Hamano wrote:

> Eric Sunshine <sunshine@sunshineco.com> writes:
> > This talks only about '=='.
> 
> Yup.  The text would need a matching change, though.

Here's a re-roll with the necessary changes.

Show 9 quoted lines

> >     if (!ptr)
> >         BUG("...");
> >     if (cnt)
> >         foo(ptr, cnt);
> 
> Or more succinctly:
> 
> 	if (!ptr || cnt)
> 		BUG("we must have an empty array at this point");

I considered that but thought it might be too "cute", however, seeing it written out, it looks fine, so I used it in the re-roll.

> > Also, would you want to talk about not comparing against NUL character?
> 
> Yeah, it might be worth saying it explicitly.  I dunno.

Rather than giving this a separate example in the re-roll, I just mentioned '\0' in the text.

--- >8 ---

From: Junio C Hamano <gitster@pobox.com>
Subject: [PATCH] CodingGuidelines: do not ==/!= compare with 0 or '\0' or NULL

Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---
 Documentation/CodingGuidelines | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
index 390ceece52..6dfc47ed7d 100644
--- a/Documentation/CodingGuidelines
+++ b/Documentation/CodingGuidelines
@@ -236,6 +236,18 @@ For C programs:
         while( condition )
 		func (bar+1);
 
+ - Do not explicitly compare an integral value with constant 0 or '\0',
+   or a pointer value with constant NULL.  For instance, to validate a
+   counted array ptr that has cnt elements, write:
+
+	if (!ptr || cnt)
+		BUG("empty array expected");
+
+   and not:
+
+	if (ptr == NULL || cnt != 0);
+		BUG("empty array expected");
+
  - We avoid using braces unnecessarily.  I.e.
 
 	if (bla) {

-- 
2.26.2.717.g5cccb0e1a8