From: | Daniel Gustafsson <daniel(at)yesql(dot)se> |
---|---|
To: | Evgeniy Gorbanev <gorbanyoves(at)basealt(dot)ru> |
Cc: | pgsql-hackers(at)lists(dot)postgresql(dot)org |
Subject: | Re: No error checking when reading from file using zstd in pg_dump |
Date: | 2025-06-16 09:00:47 |
Message-ID: | 522DB522-3962-4F91-A691-2DE790F6CCD6@yesql.se |
Views: | Whole Thread | Raw Message | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
> On 16 Jun 2025, at 10:52, Evgeniy Gorbanev <gorbanyoves(at)basealt(dot)ru> wrote:
>
> 16.06.2025 14:25, Daniel Gustafsson пишет:
>>> On 16 Jun 2025, at 10:14, Evgeniy Gorbanev <gorbanyoves(at)basealt(dot)ru> wrote:
>>> In src/bin/pg_dump/compress_zstd.c, the Zstd_read function always
>>> returns true. But if you look at the Zstd_gets and Zstd_getc functions,
>>> where Zstd_read is called via CFH->read_func, it is expected that
>>> the Zstd_read function can also return false. In case of
>>> a read-from-file error, the process is expected to terminate, but
>>> pg_dump will continue the process.
>>> I assume that after checking
>>> if (cnt == 0)
>>> should be
>>> return false;
>> if (cnt == 0)
>> - break;
>> + return false;
>>
>> As cnt is coming from fread() returning false here would be wrong as you cannot
>> from 0 alone know if it's EOF or an error. Instead it needs to inspect the
>> stream with feof() and ferror() to know how to proceed.
>
> The feof() check is in the calling functions, e.g. in the Zstd_getc
> function.
That function is using feof/ferror to log an appropriate error message, what
you are proposing is to consider all cases of EOF as an error. If you test
that you will see lots of test starting to fail.
--
Daniel Gustafsson
From | Date | Subject | |
---|---|---|---|
Next Message | Hayato Kuroda (Fujitsu) | 2025-06-16 09:11:27 | RE: Slot's restart_lsn may point to removed WAL segment after hard restart unexpectedly |
Previous Message | Vaibhav Dalvi | 2025-06-16 08:59:23 | Re: pg_upgrade fails with an error "object doesn't exist" |