Cleanup - adjust the code crossing 80-column window limit
Hi,
Attached patch makes an adjustment to ipc.c code to be in the 80-column
window.
Regards,
Amul
Attachments:
0001-cleanup-adjust-code-in-80-column-window.patchapplication/octet-stream; name=0001-cleanup-adjust-code-in-80-column-window.patchDownload
From 3657df97f5a38ca09d078b9798cc561eeea4b628 Mon Sep 17 00:00:00 2001
From: Amul Sul <amul.sul@enterprisedb.com>
Date: Wed, 1 Jul 2020 02:51:37 -0400
Subject: [PATCH] cleanup - adjust code in 80-column window
---
src/backend/storage/ipc/ipc.c | 21 +++++++++++++++------
1 file changed, 15 insertions(+), 6 deletions(-)
diff --git a/src/backend/storage/ipc/ipc.c b/src/backend/storage/ipc/ipc.c
index bdbc2c3ac4b..df28c3141cc 100644
--- a/src/backend/storage/ipc/ipc.c
+++ b/src/backend/storage/ipc/ipc.c
@@ -206,8 +206,11 @@ proc_exit_prepare(int code)
* possible.
*/
while (--on_proc_exit_index >= 0)
- on_proc_exit_list[on_proc_exit_index].function(code,
- on_proc_exit_list[on_proc_exit_index].arg);
+ {
+ struct ONEXIT onexit = on_proc_exit_list[on_proc_exit_index];
+
+ onexit.function(code, onexit.arg);
+ }
on_proc_exit_index = 0;
}
@@ -236,8 +239,11 @@ shmem_exit(int code)
elog(DEBUG3, "shmem_exit(%d): %d before_shmem_exit callbacks to make",
code, before_shmem_exit_index);
while (--before_shmem_exit_index >= 0)
- before_shmem_exit_list[before_shmem_exit_index].function(code,
- before_shmem_exit_list[before_shmem_exit_index].arg);
+ {
+ struct ONEXIT onexit = before_shmem_exit_list[before_shmem_exit_index];
+
+ onexit.function(code, onexit.arg);
+ }
before_shmem_exit_index = 0;
/*
@@ -269,8 +275,11 @@ shmem_exit(int code)
elog(DEBUG3, "shmem_exit(%d): %d on_shmem_exit callbacks to make",
code, on_shmem_exit_index);
while (--on_shmem_exit_index >= 0)
- on_shmem_exit_list[on_shmem_exit_index].function(code,
- on_shmem_exit_list[on_shmem_exit_index].arg);
+ {
+ struct ONEXIT onexit = on_shmem_exit_list[on_shmem_exit_index];
+
+ onexit.function(code, onexit.arg);
+ }
on_shmem_exit_index = 0;
shmem_exit_inprogress = false;
--
2.18.0
changes look good to me.
one comment: instead of having block variables onexit, in the while
loops in shmem_exit, can we have a single local variable defined at
the start of the shmem_exit function
and reuse them in the while loops? same comment for onexit block
variable in proc_exit_prepare() function.
Patch applies successfully on commit - 4315e8c23b9a897e12fcf91de7bfd734621096bf
make check and make check-world runs are clean.
With Regards,
Bharath Rupireddy.
EnterpriseDB: http://www.enterprisedb.com
Show quoted text
On Wed, Jul 1, 2020 at 12:31 PM Amul Sul <sulamul@gmail.com> wrote:
Hi,
Attached patch makes an adjustment to ipc.c code to be in the 80-column window.
Regards,
Amul
On Wed, Jul 1, 2020 at 4:29 PM Bharath Rupireddy
<bharath.rupireddyforpostgres@gmail.com> wrote:
changes look good to me.
Thanks for looking at the patch.
one comment: instead of having block variables onexit, in the while
loops in shmem_exit, can we have a single local variable defined at
the start of the shmem_exit function
and reuse them in the while loops? same comment for onexit block
variable in proc_exit_prepare() function.
If you are worried about the declaration and initialization of the variable will
happen with every loop cycle then you shouldn't because that happens only
once before the loop-block is entered.
Regards,
Amul
one comment: instead of having block variables onexit, in the while
loops in shmem_exit, can we have a single local variable defined at
the start of the shmem_exit function
and reuse them in the while loops? same comment for onexit block
variable in proc_exit_prepare() function.If you are worried about the declaration and initialization of the variable will
happen with every loop cycle then you shouldn't because that happens only
once before the loop-block is entered.
thanks. understood (just for info [1]https://stackoverflow.com/questions/29785789/why-do-the-objects-created-in-a-loop-have-the-same-address/29785868) .
Is there a test case covering this part of the code(I'm not sure if
one exists in the regression test suite)?
If no, can we add one?
With Regards,
Bharath Rupireddy.
EnterpriseDB: http://www.enterprisedb.com
On 2020-07-01 09:00, Amul Sul wrote:
Attached patch makes an adjustment to ipc.c code to be in the 80-column
window.
I can see an argument that this makes the code a bit easier to read, but
making code fit into 80 columns doesn't have to be a goal by itself.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Fri, Jul 3, 2020 at 1:32 PM Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:
On 2020-07-01 09:00, Amul Sul wrote:
Attached patch makes an adjustment to ipc.c code to be in the 80-column
window.I can see an argument that this makes the code a bit easier to read, but
making code fit into 80 columns doesn't have to be a goal by itself.
I wouldn't disagree with that. I believe the 80 column rule has been documented
for the code readability.
Regards,
Amul
Is there a test case covering this part of the code(I'm not sure if
one exists in the regression test suite)?
If no, can we add one?
I observed that the code areas this patch is trying to modify are
pretty much generic and are being called from many places.
The code basically handles exit callbacks on proc exits, on or before
shared memory exits which is very generic and common code.
I'm sure these parts are covered with the existing regression test suites.
Since I have previously run the regression tests, now finally, +1 for
the patch from my end.
With Regards,
Bharath Rupireddy.
EnterpriseDB: http://www.enterprisedb.com