Re: Add SHELL_EXIT_CODE to psql

From: Maxim Orlov <orlovmg(at)gmail(dot)com>
To: Corey Huinker <corey(dot)huinker(at)gmail(dot)com>
Cc: vignesh C <vignesh21(at)gmail(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: Add SHELL_EXIT_CODE to psql
Date: 2023-01-09 15:01:37
Message-ID: CACG=ezYh-=L78ZuyXU3FjCMbHXa-6_q1GhEirQifia7-icGWKQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi!

In overall, I think we move in the right direction. But we could make code
better, should we?

+ /* Capture exit code for SHELL_EXIT_CODE */
+ close_exit_code = pclose(fd);
+ if (close_exit_code == -1)
+ {
+ pg_log_error("%s: %m", cmd);
+ error = true;
+ }
+ if (WIFEXITED(close_exit_code))
+ exit_code=WEXITSTATUS(close_exit_code);
+ else if(WIFSIGNALED(close_exit_code))
+ exit_code=WTERMSIG(close_exit_code);
+ else if(WIFSTOPPED(close_exit_code))
+ exit_code=WSTOPSIG(close_exit_code);
+ if (exit_code)
+ error = true;
I think, it's better to add spaces around middle if block. It will be easy
to read.
Also, consider, adding spaces around assignment in this block.

+ /*
+ snprintf(exit_code_buf, sizeof(exit_code_buf), "%d",
WEXITSTATUS(exit_code));
+ */
Probably, this is not needed.

> 1. pg_regress now creates an environment variable called PG_OS_TARGET
Maybe, we can use env "OS"? I do not know much about Windows, but I think
this is kind of standard environment variable there.

--
Best regards,
Maxim Orlov.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2023-01-09 15:06:12 Re: Fix pg_publication_tables to exclude generated columns
Previous Message Ashutosh Bapat 2023-01-09 14:50:47 Re: Timeout when changes are filtered out by the core during logical replication