Hi
On 01.11.2020 12:37, Daniil Zakhlystov wrote:
Hi,
I have a couple of comments regarding the last patch, mostly these are minor issues.
In src/backend/libpq/pqcomm.c, starting from the line 1114:
int
pq_getbyte_if_available(unsigned char *c)
{
int r;
Assert(PqCommReadingMsg);
if (PqRecvPointer < PqRecvLength || (0) > 0) // not easy to understand optimization (maybe add a comment?)
{
*c = PqRecvBuffer[PqRecvPointer++];
return 1;
}
return r; // returned value is not initialized
}
Sorry, I don't understand it.
This is the code of pq_getbyte_if_available:
int
pq_getbyte_if_available(unsigned char *c)
{
int r;
Assert(PqCommReadingMsg);
if (PqRecvPointer < PqRecvLength || (r = pq_recvbuf(true)) > 0)
{
*c = PqRecvBuffer[PqRecvPointer++];
return 1;
}
return r;
}
So "return r" branch is executed when both conditions are false: (PqRecvPointer < PqRecvLength)
and ((r = pq_recvbuf(true)) > 0)
Last condition cause assignment of "r" variable.
I wonder how did you get this "returned value is not initialized" warning?
Is it produced by some static analyze tool or compiler?
In any case, I will initialize "r" variable to make compiler happy.
In src/interfaces/libpq/fe-connect.c, starting from the line 3255:
pqGetc(&algorithm, conn);
impl = zpq_get_algorithm_impl(algorithm);
{ // I believe that if (impl < 0) condition is missing here, otherwise there is always an error
appendPQExpBuffer(&conn->errorMessage,
libpq_gettext(
"server is not supported requested compression algorithm %c\n"), algorithm);
goto error_return;
}
Sorry I have fixed this mistyping several days ago in GIT repository
git@github.com:postgrespro/libpq_compression.git but did;t attach new version of the patch because I plan to make more changes as a result of Andres review.
In configure, starting from the line 1587:
--without-zlib do not use Zlib
--with-zstd do not use zstd // is this correct?
Thank you for noting it: fixed.
