Andrew Dunstan <andrew@dunslane.net> writes:
> Submitted for review.
Okay, some random comments:
> ! char *ListenAddresses = "localhost";
I think you made this mistake in the log_line_prefix patch too. The
contents of a GUC string var should always be either NULL or a pointer
to a malloc'd string --- ergo, its initial state must be NULL. It's
pure luck that GUC doesn't dump core trying to free() this pointer.
> + /*
> + * check if ListenAddresses is empty or all spaces
> + */
Why do you need this test (or the NetServer bool) at all? Just scan
the string and bind to whatever it mentions.
BTW it'd be better to use isspace() instead of a hardwired test for ' '.
> ! if (strcmp(ListenAddresses,"*") != 0)
This seems a bit nonrobust since it will fail if any whitespace is
added. I think it'd be better to test whether any individual hostname
extracted from the string is '*'.
> + if (ListenSocket[0] == -1)
> + ereport(FATAL,
> + (errmsg("not listening on any socket")));
Good but I don't like the wording of the error very much. Maybe "could
not bind to any socket"? Doesn't seem quite right either. [thinks...]
There are really two cases here:
* listen_addresses is empty and the machine doesn't have Unix
sockets
* listen_addresses contains (only) bogus addresses, and the
machine doesn't have Unix sockets.
"not listening on any socket" seems to focus on the first case, "could
not bind to any socket" focuses on the second. Can we cover both?
Please revise, and update the docs too.
regards, tom lane
