Tom Lane wrote:
>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.
>
Noted. Thanks.
>
>
>
>>+ /*
>>+ * 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.
>
It is used in the existing code to test if we can do SSL.
>
>BTW it'd be better to use isspace() instead of a hardwired test for ' '.
>
>
Again, the existing code for VirtualHost uses hardcoded space. I don't
mind adjusting it, but was following style in the surrounding code.
>
>
>>! 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 '*'.
>
Agree with the first point, disagree with the second. What does it mean
to specify "12.34.56.78 *"? I think "*" should be allowed only if it
is the only entry in the list.
>
>
>
>>+ 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?
>
I will think about it. Bear in mind that they will have already had
warnings about specific failures.
>
>Please revise, and update the docs too.
>
Will do.
Thanks
andrew