From: Tom Lane <tgl@sss.pgh.pa.us>
To: Fabien COELHO <coelho@cri.ensmp.fr>
cc: Peter Eisentraut <peter.eisentraut@enterprisedb.com>,
        Michael Paquier <michael@paquier.xyz>,
        Julien Rouhaud <rjuju123@gmail.com>,
        "shiy.fnst@fujitsu.com" <shiy.fnst@fujitsu.com>,
        "pgsql-hackers@lists.postgresql.org"
 <pgsql-hackers@lists.postgresql.org>
Subject: Re: psql - add SHOW_ALL_RESULTS option
In-reply-to: <2671235.1618154047@sss.pgh.pa.us>
References: <1ec5ef49-b5c8-4129-e680-fe42f8d0670b@2ndquadrant.com>
 <alpine.DEB.2.22.394.2104062323150.2555247@pseudo>
 <OSZPR01MB631017521EE6887ADC9492E8FD759@OSZPR01MB6310.jpnprd01.prod.outlook.com>
 <alpine.DEB.2.22.394.2104071516470.2724014@pseudo>
 <OSZPR01MB6310325023E8B4109BB2AB90FD749@OSZPR01MB6310.jpnprd01.prod.outlook.com>
 <20210408163050.fca7dwb37zszbsmm@nol>
 <alpine.DEB.2.22.394.2104081858500.3192110@pseudo>
 <20210408171135.ekhdf5nvqss554q3@nol> <YG+okFpc3JaKN/zB@paquier.xyz>
 <alpine.DEB.2.22.394.2104090828330.3253600@pseudo>
 <YHAxrQPk2E62wQSS@paquier.xyz>
 <alpine.DEB.2.22.394.2104091947240.3439700@pseudo>
 <2671235.1618154047@sss.pgh.pa.us>
Comments: In-reply-to Tom Lane <tgl@sss.pgh.pa.us>
	message dated "Sun, 11 Apr 2021 11:14:07 -0400"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <2680033.1618157764.1@sss.pgh.pa.us>
Content-Transfer-Encoding: quoted-printable
Date: Sun, 11 Apr 2021 12:16:04 -0400
Message-ID: <2680034.1618157764@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/2680034.1618157764%40sss.pgh.pa.us>
Precedence: bulk

... btw, Coverity also doesn't like this fragment of the patch:

/srv/coverity/git/pgsql-git/postgresql/src/bin/psql/common.c: 1084 in Show=
NoticeMessage()
1078     static void
1079     ShowNoticeMessage(t_notice_messages *notes)
1080     {
1081     	PQExpBufferData	*current =3D notes->in_flip ? &notes->flip : &no=
tes->flop;
1082     	if (current->data !=3D NULL && *current->data !=3D '\0')
1083     		pg_log_info("%s", current->data);
>>>     CID 1476041:  Null pointer dereferences  (FORWARD_NULL)
>>>     Passing "current" to "resetPQExpBuffer", which dereferences null "=
current->data".
1084     	resetPQExpBuffer(current);
1085     }
1086     =

1087     /*
1088      * SendQueryAndProcessResults: utility function for use by SendQu=
ery()
1089      * and PSQLexecWatch().

Its point here is that either the test of "current->data !=3D NULL" is
useless, or resetPQExpBuffer needs such a test too.  I'm inclined
to guess the former.

(Just as a matter of style, I don't care for the flip/flop terminology
here, not least because it's not clear why exactly two buffers suffice
and will suffice forevermore.  I'd be inclined to use an array of
two buffers with an index variable.)

			regards, tom lane