Re: libpq compression

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(dot)knizhnik(at)postgrespro(dot)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
> git(at)github(dot)com:postgrespro/libpq_compression.git <mailto:git(at)github(dot)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>