Re: backup manifests - Mailing list pgsql-hackers

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@gmail.com> wrote:

On Thu, Mar 12, 2020 at 10:47 AM tushar <tushar.ahuja@enterprisedb.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.