Re: basebackup.c's sendFile() ignores read errors

Jeevan Chalke <jeevan.chalke@enterprisedb.com>

From: Jeevan Chalke <jeevan.chalke@enterprisedb.com>

To: Jeevan Ladhe <jeevan.ladhe@enterprisedb.com>

Cc: Robert Haas <robertmhaas@gmail.com>, "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>

Date: 2019-08-30T10:34:43Z

Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

When performing a base backup, check for read errors.
- fbe897134801 9.4.25 landed
- f697c6396d73 9.5.20 landed
- 23df8822601d 9.6.16 landed
- 62fb12d7e138 10.11 landed
- 61c65cce40e6 11.6 landed
- ce5542d40253 12.0 landed
- 286af0ce1211 13.0 landed

Attachments

fread_error_check_v2.patch (text/x-patch) patch v2

On Thu, Aug 29, 2019 at 3:17 PM Jeevan Ladhe <jeevan.ladhe@enterprisedb.com>
wrote:

> Hi Jeevan
>
> I had a look at the patch and this seems correct to me.
>

Thanks, Jeevan Ladhe.


>
> Few minor comments:
>
> + /* Check fread() error. */
> + CHECK_FREAD_ERROR(fp, pathbuf);
> +
>
> The comments above the macro call at both the places are not necessary as
> your macro name itself is self-explanatory.
>
> ----------
> + /*
> + * If file is truncated, then we will hit
> + * end-of-file error in which case we don't
> + * want to error out, instead just pad it with
> + * zeros.
> + */
> + if (feof(fp))
>
> The if block does not do the truncation right away, so I think the comment
> above can be reworded to explain why we reset cnt?
>

Fixed both comments in the attached patch.

-- 
Jeevan Chalke
Technical Architect, Product Development
EnterpriseDB Corporation
The Enterprise PostgreSQL Company