On 2025-Apr-15, Mahendra Singh Thalor wrote:
> I took this patch and did some testing. Patch looks good to me.
Hello, thank you very much for looking.
> I was not able to use "git am" in my setup due to CR's in patch but no
> warning in the patch when I used "patch -p".
Hmm, don't blame me; rather I think your email rig is broken and
produces CRs for no reason (pretty normal on Windows I think). Here's
what hexdump shows for the attached file direct from the email:
000002a0 2d 2d 2d 20 61 2f 73 72 63 2f 62 69 6e 2f 70 67 |--- a/src/bin/pg|
000002b0 5f 64 75 6d 70 2f 70 67 5f 72 65 73 74 6f 72 65 |_dump/pg_restore|
000002c0 2e 63 0a 2b 2b 2b 20 62 2f 73 72 63 2f 62 69 6e |.c.+++ b/src/bin|
000002d0 2f 70 67 5f 64 75 6d 70 2f 70 67 5f 72 65 73 74 |/pg_dump/pg_rest|
000002e0 6f 72 65 2e 63 0a 40 40 20 2d 36 37 2c 31 30 20 |ore.c.@@ -67,10 |
000002f0 2b 36 37 2c 32 30 20 40 40 20 73 74 61 74 69 63 |+67,20 @@ static|
00000300 20 69 6e 74 09 70 72 6f 63 65 73 73 5f 67 6c 6f | int.process_glo|
00000310 62 61 6c 5f 73 71 6c 5f 63 6f 6d 6d 61 6e 64 73 |bal_sql_commands|
00000320 28 50 47 63 6f 6e 6e 20 2a 63 6f 6e 6e 2c 20 63 |(PGconn *conn, c|
00000330 6f 6e 73 74 20 63 68 61 72 20 2a 64 75 6d 70 64 |onst char *dumpd|
00000340 69 72 70 61 74 68 2c 0a 20 09 09 09 09 09 09 09 |irpath,. .......|
00000350 09 09 09 63 6f 6e 73 74 20 63 68 61 72 20 2a 6f |...const char *o|
00000360 75 74 66 69 6c 65 29 3b 0a 20 73 74 61 74 69 63 |utfile);. static|
In bytes 340ff you can see that the comma (2c) is followed by 0a ("line
feed" or newline) and then a bunch of tabs (09). There's no carriage
return (0d) anywhere.
Maybe you would be able to convince git not to complain by downloading
the file from the email[1] directly onto it somehow, without letting
Windows mess things up. Maybe something like this on a command line
curl
"https://www.postgresql.org/message-id/attachment/175873/0001-remove-unnecessary-oid_string-list-stuff.patch";
| git am -
assuming you can get curl to run on a command line at all.
[1] https://postgr.es/m/202504141220.343fmoxfsbj4@alvherre.pgsql
> *One review comment:*
> We are using FLEXIBLE_ARRAY_MEMBER for dbname. Can we use NAMEDATALEN? In
> code, many places we are using this hard coded value of 64 size for names.
Well, yes we can, but then we'd be allocating a bunch of null bytes for
no reason. The current allocation strategy, which comes from the
original commit not my code, is to allocate a struct with the OID and
just enough bytes to store the database name, which is known. I don't
see any reason to modify this tbh. The reason to use NAMEDATALEN in
various places is to have enough room to store whatever name the user
specifies, without having to know the length before the allocation
occurs.
FLEXIBLE_ARRAY_MEMBER is there to document exactly what's happening
here, and used thoroughly across the codebase.
--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
"E pur si muove" (Galileo Galilei)
