Re: Patch to allow pg_filedump to support reading of pg_filenode.map - Mailing list pgsql-hackers

From Richard Yen
Subject Re: Patch to allow pg_filedump to support reading of pg_filenode.map
Date
Msg-id CAKH4vDiUK24NNf+JmD8yc5vL57A3MfGNpM4mQqH7c9C590Ro1w@mail.gmail.com
Whole thread Raw
In response to Re: Patch to allow pg_filedump to support reading of pg_filenode.map  (Justin Pryzby <pryzby@telsasoft.com>)
Responses Re: Patch to allow pg_filedump to support reading of pg_filenode.map
List pgsql-hackers
Thanks for the feedback, Justin.  I've gone ahead and switched to use memcmp.  I also refactored it to:

1. Don't assume that any file with first 4 bytes matching the relmapper magic number is a pg_relnode.map file
2. Don't assume the pg_relnode.map file is uncorrupted and intact; perform a check of the first 4 bytes against the reference magic number
3. Provide a flag (-m) for users to have their file interpreted as a pg_relnode.map file

I hope this is more palatable to everyone :)

--Richard



On Wed, Apr 28, 2021 at 9:42 PM Justin Pryzby <pryzby@telsasoft.com> wrote:
This is separate from the postgresql server repo.
https://git.postgresql.org/gitweb/?p=pg_filedump.git

+#define RELMAPPER_FILEMAGIC   0x592717
+char magic_buffer[8];

...

+  if ( (int) magic_buffer & RELMAPPER_FILEMAGIC ) {

This is doing bitwise arithmetic on a pointer, which seems badly wrong.
I think it breaks normal use of pg_filedump - unless you happen to get a
magic_buffer without those bits set.  The segfault seems to confirm that, as
does gcc:

pg_filedump.c:2041:8: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
 2041 |   if ( (int) magic_buffer & RELMAPPER_FILEMAGIC ) {

I think it probably means to do memcmp, instead ??

--
Justin
Attachment

pgsql-hackers by date:

Previous
From: Álvaro Herrera
Date:
Subject: Re: Race condition in InvalidateObsoleteReplicationSlots()
Next
From: Alvaro Herrera
Date:
Subject: Re: Remove redundant variable from transformCreateStmt