Re: backup manifests

One more suggestion, recent commit (1933ae62) has added the PostgreSQL home
page to --help output.

e.g:
*PostgreSQL home page: <https://www.postgresql.org/
<https://www.postgresql.org/>>*

We might need to consider this change for pg_validatebackup binary.

On Mon, Mar 16, 2020 at 10:37 AM Suraj Kharage <
suraj(dot)kharage(at)enterprisedb(dot)com> wrote:

> Thank you, Robert.
>
> Getting below warning while compiling the
> v11-0003-pg_validatebackup-Validate-a-backup-against-the-.patch.
>
>
>
> *pg_validatebackup.c: In function
> ‘report_manifest_error’:pg_validatebackup.c:356:2: warning: function might
> be possible candidate for ‘gnu_printf’ format attribute
> [-Wsuggest-attribute=format] pg_log_generic_v(PG_LOG_FATAL, fmt, ap);*
>
>
> To resolve this, can we use "pg_attribute_printf(2, 3)" in function
> declaration something like below?
> e.g:
>
> diff --git a/src/bin/pg_validatebackup/parse_manifest.h
> b/src/bin/pg_validatebackup/parse_manifest.h
> index b0b18a5..25d140f 100644
> --- a/src/bin/pg_validatebackup/parse_manifest.h
> +++ b/src/bin/pg_validatebackup/parse_manifest.h
> @@ -25,7 +25,7 @@ typedef void
> (*json_manifest_perfile_callback)(JsonManifestParseContext *,
> size_t
> size, pg_checksum_type checksum_type,
> int
> checksum_length, uint8 *checksum_payload);
> typedef void (*json_manifest_error_callback)(JsonManifestParseContext *,
> - char
> *fmt, ...);
> + char
> *fmt,...) pg_attribute_printf(2, 3);
>
> struct JsonManifestParseContext
> {
> diff --git a/src/bin/pg_validatebackup/pg_validatebackup.c
> b/src/bin/pg_validatebackup/pg_validatebackup.c
> index 0e7299b..6ccbe59 100644
> --- a/src/bin/pg_validatebackup/pg_validatebackup.c
> +++ b/src/bin/pg_validatebackup/pg_validatebackup.c
> @@ -95,7 +95,7 @@ static void
> record_manifest_details_for_file(JsonManifestParseContext *context,
>
> int checksum_length,
>
> uint8 *checksum_payload);
> static void report_manifest_error(JsonManifestParseContext *context,
> - char
> *fmt, ...);
> + char
> *fmt,...) pg_attribute_printf(2, 3);
>
> static void validate_backup_directory(validator_context *context,
>
> char *relpath, char *fullpath);
>
>
> Typos:
>
> 0004 patch
> unexpctedly => unexpectedly
>
> 0005 patch
> bacup => backup
>
> On Sat, Mar 14, 2020 at 2:04 AM Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>
>> On Thu, Mar 12, 2020 at 10:47 AM tushar <tushar(dot)ahuja(at)enterprisedb(dot)com>
>> wrote:
>> > On 3/9/20 10:46 PM, Robert Haas wrote:
>> > > Seems like expected behavior to me. We could consider providing a more
>> > > descriptive error message, but there's now way for it to work.
>> >
>> > Right , Error message need to be more user friendly .
>>
>> OK. Done in the attached version, which also includes a few other changes:
>>
>> - I expanded the regression tests. They now cover every line of code
>> in parse_manifest.c except for a few that I believe to be unreachable
>> (though I might be mistaken). Coverage for pg_validatebackup.c is also
>> improved, but it's not 100%; there are some cases that I don't know
>> how to hit outside of a kernel malfunction, and others that I only
>> know how to hit on non-Windows systems. For instance, it's easy to use
>> perl to make a file inaccessible on Linux with chmod(0, $filename),
>> but I gather that doesn't work on Windows. I'm going to spend a bit
>> more time looking at this, but I think it's already reasonably good.
>>
>> - I fixed a couple of very minor bugs which I discovered by writing those
>> tests.
>>
>> - I added documentation, in part based on a draft Mark Dilger shared
>> with me off-list.
>>
>> I don't think this is committable just yet, but I think it's getting
>> fairly close, so if anyone has major objections please speak up soon.
>>
>> --
>> Robert Haas
>> EnterpriseDB: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>
>
> --
> --
>
> Thanks & Regards,
> Suraj kharage,
> EnterpriseDB Corporation,
> The Postgres Database Company.
>

--
--

Thanks & Regards,
Suraj kharage,
EnterpriseDB Corporation,
The Postgres Database Company.