Date: Mon, 28 Mar 2022 19:07:27 -0500
From: Justin Pryzby <pryzby@telsasoft.com>
To: Robert Haas <robertmhaas@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>,
	Dagfinn Ilmari =?iso-8859-1?Q?Manns=E5ker?= <ilmari@ilmari.org>,
	Dipesh Pandit <dipesh.pandit@gmail.com>,
	Andres Freund <andres@anarazel.de>,
	Jeevan Ladhe <jeevanladhe.os@gmail.com>,
	Alvaro Herrera <alvherre@alvh.no-ip.org>,
	"Shinoda, Noriyoshi (PN Japan FSIP)" <noriyoshi.shinoda@hpe.com>,
	Abhijit Menon-Sen <ams@toroid.org>,
	Dmitry Dolgov <9erthalion6@gmail.com>,
	Jeevan Ladhe <jeevan.ladhe@enterprisedb.com>,
	Mark Dilger <mark.dilger@enterprisedb.com>,
	pgsql-hackers@postgresql.org,
	tushar <tushar.ahuja@enterprisedb.com>
Subject: Re: refactoring basebackup.c (zstd workers)
Message-ID: <20220329000727.GV28503@telsasoft.com>
References: <20220321131833.GB28503@telsasoft.com>
 <CA+TgmoZz-F1pA1Vv=-6FOPWuS1eFi2ifNyySV4u6pW0+FBeG-g@mail.gmail.com>
 <20220321182256.GC28503@telsasoft.com>
 <CA+TgmoYJ=9Y=+fJ9WyAXAExTmpArCZEdRu1iVQ_2je+heTZGrg@mail.gmail.com>
 <871qyvrvvm.fsf@wibble.ilmari.org>
 <CA+Tgmobummc+reyzZJwnua7KXPC9sromDqYx1uoZ49mxq=LRfQ@mail.gmail.com>
 <384291.1648403267@sss.pgh.pa.us>
 <CA+TgmoZ1DbW_nKeVteVvYt0WvZDhm77_qdb-TErjctW+h5_3Jg@mail.gmail.com>
 <20220328205329.GT28503@telsasoft.com>
 <CA+TgmoYw=UFomz2f8Uq0LtUmvcReRFqKb19YwO2O0Wv0Azz_ng@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: 
 <CA+TgmoYw=UFomz2f8Uq0LtUmvcReRFqKb19YwO2O0Wv0Azz_ng@mail.gmail.com>
User-Agent: Mutt/1.9.4 (2018-02-28)
Archived-At: 
 <https://www.postgresql.org/message-id/20220329000727.GV28503%40telsasoft.com>
Precedence: bulk

On Mon, Mar 28, 2022 at 05:39:31PM -0400, Robert Haas wrote:
> On Mon, Mar 28, 2022 at 4:53 PM Justin Pryzby <pryzby@telsasoft.com> wrote:
> > I suggest to write it differently, as in 0002.
> 
> That doesn't seem better to me. What's the argument for it?

I find this much easier to understand:

                /* If we got an error or have reached the end of the string, stop. */                                                                                                                                              
-               if (result->parse_error != NULL || *kwend == '\0' || *vend == '\0')                                                                                                                                                
+               if (result->parse_error != NULL)                                                                                                                                                                                   
+                       break;                                                                                                                                                                                                     
+               if (*kwend == '\0')                                                                                                                                                                                                
+                       break;                                                                                                                                                                                                     
+               if (vend != NULL && *vend == '\0')                                                                                                                                                                                 
                        break;                                                                                                                                                                                                     

than

                /* If we got an error or have reached the end of the string, stop. */
-               if (result->parse_error != NULL || *kwend == '\0' || *vend == '\0')
+               if (result->parse_error != NULL ||
+                       (vend == NULL ? *kwend == '\0' : *vend == '\0'))

Also, why wouldn't *kwend be checked in any case ?