+ /* FIXME need to check that port is numeric */
Is this still applicable?.
1)
+ /*
+ * if there is more than one host in the connect string and
+ * target_server_type is not specified explicitely, set
+ * target_server_type to "master"
+ */
I think we need comments to know why we change default value based on number of elements in connection string. why default in not “any" when node count > 1.
2)
+ /* loop over all the host specs in the node variable */
+ for (node = nodes; node->host != NULL || node->port != NULL; node++)
{
I think instead of loop if we can move these code into a separate function say pg_add_to_addresslist(host, port, &addrs) this helps in removing temp variables like "struct node info” and several heap calls around it.
3)
+static char *
+get_port_from_addrinfo(struct addrinfo * ai)
+{
+ char port[6];
+
+ sprintf(port, "%d", htons(((struct sockaddr_in *) ai->ai_addr)->sin_port));
+ return strdup(port);
+}
Need some comments for this function.
We use strdup in many places no where we handle memory allocation failure.
4)
+ /*
+ * consult connection options and check if RO connection is OK RO
+ * connection is OK if readonly connection is explicitely
+ * requested or if replication option is set, or just one host is
+ * specified in the connect string.
+ */
+ if (conn->pghost == NULL || !conn->target_server_type ||
+ strcmp(conn->target_server_type, "any") == 0)
Comments not in sink with code.
