| From: | "Greg Burd" <greg(at)burd(dot)me> |
|---|---|
| To: | "Tom Lane" <tgl(at)sss(dot)pgh(dot)pa(dot)us>, "PostgreSQL Hackers" <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Cc: | "Thomas Munro" <thomas(dot)munro(at)gmail(dot)com> |
| Subject: | Re: Areas for Solaris support modernization |
| Date: | 2026-03-02 19:48:11 |
| Message-ID: | 2cbf958e-a36f-4121-b9e9-13ebaaa1cb2c@app.fastmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Sun, Mar 1, 2026, at 9:50 PM, Tom Lane wrote:
> I wrote:
>> Also, while playing with said local OpenIndiana image, I noticed
>> that ps_status.c isn't working: "ps auxww" shows all the child
>> processes with the same command line as the postmaster. I thought
>> maybe we'd diked out something important in d2ea2d310, but none
>> of the code removed there claims to apply to Solaris. So maybe
>> it never worked on Solaris? Anyway, there's room for improvement
>> there if anyone cares to investigate.
Hey Tom,
Thanks for digging into this, it's been on my mind for a while to get back to "tending to the animals".
> Huh: after a bit of testing, it seems that the PS_USE_CHANGE_ARGV
> mode removed by d2ea2d310 is indeed the right thing to use on
> Solaris. Looking back at that discussion thread, we were a bit
> confused about which predefined macros are provided on Solaris.
> To believe that the pre-existing code actually worked on Solaris,
> you'd have to assume that "BSD" is predefined on that platform.
> It does not get defined on my OpenIndiana image, but maybe sometime
> in the stone age Solaris defined it?
>
> Anyway, here's a slightly cleaned-up reversion of the relevant
> bits of d2ea2d310, with PS_USE_CHANGE_ARGV now selected by
> "defined(__sun)" not the previous logic
> "(defined(BSD) || defined(__hurd__)) && !defined(__darwin__)".
I've started a build/test on "icarus" with your patch applied. I'll let you know how it goes, hopefully that won't take as long now.
> BTW, I notice that with this, PS_PADDING is set to '\0'
> in exactly the cases that select PS_USE_CLOBBER_ARGV.
> I'm not inclined to merge the logic, because maybe we'll
> find some weird platform where the conditions are different.
> But it seems plausible and comforting that there are
> fewer underlying behaviors than we thought.
>
> regards, tom lane
best.
-greg
> diff --git a/src/backend/utils/misc/ps_status.c
> b/src/backend/utils/misc/ps_status.c
> index 51dce24947a..f5fc7124211 100644
> --- a/src/backend/utils/misc/ps_status.c
> +++ b/src/backend/utils/misc/ps_status.c
> @@ -39,6 +39,9 @@ bool update_process_title =
> DEFAULT_UPDATE_PROCESS_TITLE;
> * PS_USE_SETPROCTITLE
> * use the function setproctitle(const char *, ...)
> * (other BSDs)
> + * PS_USE_CHANGE_ARGV
> + * assign argv[0] = "string"
> + * (Solaris)
> * PS_USE_CLOBBER_ARGV
> * write over the argv and environment area
> * (Linux and most SysV-like systems)
> @@ -52,7 +55,9 @@ bool update_process_title =
> DEFAULT_UPDATE_PROCESS_TITLE;
> #define PS_USE_SETPROCTITLE_FAST
> #elif defined(HAVE_SETPROCTITLE)
> #define PS_USE_SETPROCTITLE
> -#elif defined(__linux__) || defined(_AIX) || defined(__sun) ||
> defined(__darwin__) || defined(__GNU__)
> +#elif defined(__sun)
> +#define PS_USE_CHANGE_ARGV
> +#elif defined(__linux__) || defined(_AIX) || defined(__darwin__) ||
> defined(__GNU__)
> #define PS_USE_CLOBBER_ARGV
> #elif defined(WIN32)
> #define PS_USE_WIN32
> @@ -223,6 +228,9 @@ save_ps_display_args(int argc, char **argv)
> ps_status_new_environ = new_environ;
> #endif
> }
> +#endif /* PS_USE_CLOBBER_ARGV */
> +
> +#if defined(PS_USE_CHANGE_ARGV) || defined(PS_USE_CLOBBER_ARGV)
>
> /*
> * If we're going to change the original argv[] then make a copy for
> @@ -268,7 +276,7 @@ save_ps_display_args(int argc, char **argv)
>
> argv = new_argv;
> }
> -#endif /* PS_USE_CLOBBER_ARGV */
> +#endif /* PS_USE_CHANGE_ARGV or PS_USE_CLOBBER_ARGV */
>
> return argv;
> }
> @@ -305,7 +313,18 @@ init_ps_display(const char *fixed_part)
> /* If ps_buffer is a pointer, it might still be null */
> if (!ps_buffer)
> return;
> +#endif
>
> + /*
> + * Overwrite argv[] to point at appropriate space, if needed
> + */
> +
> +#ifdef PS_USE_CHANGE_ARGV
> + save_argv[0] = ps_buffer;
> + save_argv[1] = NULL;
> +#endif /* PS_USE_CHANGE_ARGV */
> +
> +#ifdef PS_USE_CLOBBER_ARGV
> /* make extra argv slots point at end_of_area (a NUL) */
> for (int i = 1; i < save_argc; i++)
> save_argv[i] = ps_buffer + ps_buffer_size;
>
> Attachments:
> * restore-solaris-ps_status.patch
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Nathan Bossart | 2026-03-02 19:55:05 | Re: Speed up COPY FROM text/CSV parsing using SIMD |
| Previous Message | Jacob Champion | 2026-03-02 19:31:31 | Re: [oauth] Add TLS support to OAuth tests |