Re: psql - add SHOW_ALL_RESULTS option

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr>
Cc: Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com>, Michael Paquier <michael(at)paquier(dot)xyz>, Julien Rouhaud <rjuju123(at)gmail(dot)com>, "shiy(dot)fnst(at)fujitsu(dot)com" <shiy(dot)fnst(at)fujitsu(dot)com>, "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: psql - add SHOW_ALL_RESULTS option
Date: 2021-04-11 16:16:04
Message-ID: 2680034.1618157764@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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

/srv/coverity/git/pgsql-git/postgresql/src/bin/psql/common.c: 1084 in ShowNoticeMessage()
1078 static void
1079 ShowNoticeMessage(t_notice_messages *notes)
1080 {
1081 PQExpBufferData *current = notes->in_flip ? &notes->flip : &notes->flop;
1082 if (current->data != NULL && *current->data != '\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 SendQuery()
1089 * and PSQLexecWatch().

Its point here is that either the test of "current->data != 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

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2021-04-11 16:42:58 Re: PL/R regression on windows, but not linux with master.
Previous Message Peter Geoghegan 2021-04-11 16:10:42 Re: PANIC: wrong buffer passed to visibilitymap_clear