Hi, On 2024-04-11 15:16:57 -0400, Andrew Dunstan wrote: > On 2024-04-11 Th 15:01, Andres Freund wrote: > > [1345/1940 42 69%] Compiling C object > > src/bin/pg_verifybackup/pg_verifybackup.p/pg_verifybackup.c.o > > ../../../../../home/andres/src/postgresql/src/bin/pg_verifybackup/pg_verifybackup.c: > > In function 'verify_file_checksum': > > ../../../../../home/andres/src/postgresql/src/bin/pg_verifybackup/pg_verifybackup.c:842:1: > > warning: stack usage is 131232 bytes [-Wstack-usage=] > > 842 | verify_file_checksum(verifier_context *context, manifest_file *m, > > | ^~~~~~~~~~~~~~~~~~~~ > > > This one's down to me. I asked Robert some time back why we were using a > very conservative buffer size, and he agreed we could probably make it > larger, but the number chosen is mine, not his. It was a completely > arbitrary choice. > > I'm happy to reduce it, but it's not clear to me why we care that much for a > client binary. There's no stack depth checking going on here.
There isn't - but it that doesn't necessarily mean it's great to just use a lot of stack. It can even mean that you ought to be more careful, because you'll just crash, rather than get an error about having exceeded the stack size. When the stack size is increased by more than a few pages, the OS / compiler defenses for detecting that don't work as well, if you're unlucky. 128k is probably not going to be an issue in practice. However, it also seems not great from a performance POV to use this much stack in a function that's called fairly often. I'd allocate the buffer in verify_backup_checksums() and reuse it across all to-be-checked files. Greetings, Andres Freund
