Re: Modernize const handling with readline
On 04.10.23 17:09, Aleksander Alekseev wrote: Hi, On 03.10.23 13:28, Aleksander Alekseev wrote: While examining the code for similar places I noticed that the following functions can also be const'ified: - crc32_sz I suppose this could be changed. OK, that's a simple change. Here is the patch. committed this and my patch
Re: Modernize const handling with readline
Hi, > On 03.10.23 13:28, Aleksander Alekseev wrote: > > While examining the code for similar places I noticed that the > > following functions can also be const'ified: > > > > - crc32_sz > > I suppose this could be changed. OK, that's a simple change. Here is the patch. -- Best regards, Aleksander Alekseev v1-0001-Constify-crc32_sz.patch Description: Binary data
Re: Modernize const handling with readline
On 03.10.23 13:28, Aleksander Alekseev wrote: While examining the code for similar places I noticed that the following functions can also be const'ified: - crc32_sz I suppose this could be changed. - pg_checksum_page (? temporary modifies the page but then restores it) Then it's not really const? - XLogRegisterData (?) I don't think this would work, at least without further work elsewhere, because the data is stored in XLogRecData, which has no const handling. The callers of cstring_to_text[_with_len] often cast the argument to (char *) while in fact it's (const char *). This can be refactored too. These look fine to me. Additionally there is a slight difference between XLogRegisterBlock() declaration in xloginsert.h: ``` extern void XLogRegisterBlock(uint8 block_id, RelFileLocator *rlocator, ForkNumber forknum, BlockNumber blknum, char *page, uint8 flags); ``` ... and xloginsert.c: ``` void XLogRegisterBlock(uint8 block_id, RelFileLocator *rlocator, ForkNumber forknum, BlockNumber blknum, Page page, uint8 flags) ``` It looks like the reason here is that xloginsert.h does not have the Page type in scope. I don't know how difficult it would be to change that, but it seems fine as is.
Re: Modernize const handling with readline
Peter Eisentraut writes: > The comment > /* On some platforms, readline is declared as readline(char *) */ > is obsolete. The casting away of const can be removed. +1, that's surely not of interest on anything we still support. regards, tom lane
Re: Modernize const handling with readline
Hi, > The comment > > /* On some platforms, readline is declared as readline(char *) */ > > is obsolete. The casting away of const can be removed. > > The const in the readline() prototype was added in GNU readline 4.2, > released in 2001. BSD libedit has also had const in the prototype since > at least 2001. > > (The commit that introduced this comment (187e865174) talked about > FreeBSD 4.8, which didn't have readline compatibility in libedit yet, so > it must have been talking about GNU readline in the base system. This > checks out, but already FreeBSD 5 had an updated GNU readline with const.) LGTM. While examining the code for similar places I noticed that the following functions can also be const'ified: - crc32_sz - pg_checksum_page (? temporary modifies the page but then restores it) - XLogRegisterData (?) The callers of cstring_to_text[_with_len] often cast the argument to (char *) while in fact it's (const char *). This can be refactored too. Additionally there is a slight difference between XLogRegisterBlock() declaration in xloginsert.h: ``` extern void XLogRegisterBlock(uint8 block_id, RelFileLocator *rlocator, ForkNumber forknum, BlockNumber blknum, char *page, uint8 flags); ``` ... and xloginsert.c: ``` void XLogRegisterBlock(uint8 block_id, RelFileLocator *rlocator, ForkNumber forknum, BlockNumber blknum, Page page, uint8 flags) ``` Will there be a value in addressing anything of this? -- Best regards, Aleksander Alekseev
Modernize const handling with readline
The comment /* On some platforms, readline is declared as readline(char *) */ is obsolete. The casting away of const can be removed. The const in the readline() prototype was added in GNU readline 4.2, released in 2001. BSD libedit has also had const in the prototype since at least 2001. (The commit that introduced this comment (187e865174) talked about FreeBSD 4.8, which didn't have readline compatibility in libedit yet, so it must have been talking about GNU readline in the base system. This checks out, but already FreeBSD 5 had an updated GNU readline with const.) From 4db9b54b40833486e366c2d117291147ce47b195 Mon Sep 17 00:00:00 2001 From: Peter Eisentraut Date: Tue, 3 Oct 2023 12:08:25 +0200 Subject: [PATCH] Modernize const handling with readline The comment /* On some platforms, readline is declared as readline(char *) */ is obsolete. The casting away of const can be removed. The const in the readline() prototype was added in GNU readline 4.2, released in 2001. BSD libedit has also had const in the prototype since at least 2001. (The commit that introduced this comment (187e865174) talked about FreeBSD 4.8, which didn't have readline compatibility in libedit yet, so it must have been talking about GNU readline in the base system. This checks out, but already FreeBSD 5 had an updated GNU readline with const.) --- src/bin/psql/input.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/bin/psql/input.c b/src/bin/psql/input.c index 873d85079c..7c77fd8e89 100644 --- a/src/bin/psql/input.c +++ b/src/bin/psql/input.c @@ -88,8 +88,7 @@ gets_interactive(const char *prompt, PQExpBuffer query_buf) /* Enable SIGINT to longjmp to sigint_interrupt_jmp */ sigint_interrupt_enabled = true; - /* On some platforms, readline is declared as readline(char *) */ - result = readline((char *) prompt); + result = readline(prompt); /* Disable SIGINT again */ sigint_interrupt_enabled = false; -- 2.42.0
