It looks there was a different version of pq_getbyte_if_available on my side,
my bad.
I’ve reapplied the patch and compiler is happy now, thanks!
—
Daniil Zakhlystov
> On Nov 1, 2020, at 3:01 PM, Konstantin Knizhnik <k.knizh...@postgrespro.ru>
> wrote:
>
> 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
> g...@github.com:postgrespro/libpq_compression.git
> <mailto:g...@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.
>
>
> <libpq-compression-22.patch>
