Tom Lane wrote:

Andrew Dunstan <[EMAIL PROTECTED]> 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




---------------------------(end of broadcast)--------------------------- TIP 7: don't forget to increase your free space map settings

Reply via email to