(I have also looked at previous patches in the series, but have no
comments about them)
04.12.2014 23:44, Tanu Kaskinen wrote:
+void pa_tunnel_manager_remote_server_new(pa_tunnel_manager *manager,
pa_tunnel_manager_remote_server_config *config) {
Why void? There are some failure conditions (such as bad address).
+ int r;
+ pa_parsed_address parsed_address;
+ pa_tunnel_manager_remote_server *server = NULL;
+
+ pa_assert(manager);
+ pa_assert(config);
+
+ if (!config->address) {
+ pa_log("No address configured for remote server %s.", config->name);
+ return;
+ }
+
+ r = pa_parse_address(config->address->value, &parsed_address);
+ if (r < 0) {
+ pa_log("[%s:%u] Invalid address: \"%s\"", config->address->filename,
config->address->lineno, config->address->value);
+ return;
+ }
+
+ pa_xfree(parsed_address.path_or_host);
+
+ server = pa_xnew0(pa_tunnel_manager_remote_server, 1);
+ server->manager = manager;
+ server->name = pa_xstrdup(config->name);
+ server->address = pa_xstrdup(config->address->value);
+ server->devices = pa_hashmap_new(pa_idxset_string_hash_func,
pa_idxset_string_compare_func);
+ server->device_stubs = pa_hashmap_new(NULL, NULL);
+
+ pa_assert_se(pa_hashmap_put(manager->remote_servers, server->name, server)
>= 0);
+
+ pa_log_debug("Created remote server %s.", server->name);
+ pa_log_debug(" Address: %s", server->address);
+ pa_log_debug(" Failed: %s", pa_boolean_to_string(server->failed));
How can server->failed become true here? Maybe you meant this debug
output to be placed after set_up_connection()?
+
+ set_up_connection(server);
+}
<snip>
+static void set_up_connection(pa_tunnel_manager_remote_server *server) {
+ pa_assert(server);
+ pa_assert(!server->context);
+
+ server->context = pa_context_new(server->manager->core->mainloop,
"PulseAudio");
+ if (server->context) {
+ int r;
+
+ r = pa_context_connect(server->context, server->address,
PA_CONTEXT_NOFLAGS, NULL);
+ if (r >= 0)
+ pa_context_set_state_callback(server->context, context_state_cb,
server);
+ else {
+ pa_log("[%s] pa_context_connect() failed: %s", server->name,
pa_strerror(pa_context_errno(server->context)));
+ pa_tunnel_manager_remote_server_set_failed(server, true);
+ }
+ } else {
+ pa_log("[%s] pa_context_new() failed.", server->name);
+ pa_tunnel_manager_remote_server_set_failed(server, true);
+ }
+}
Here one can reduce the need for "else" statements and thus increase
readability by handling the error cases first. I.e.: if
(!server->context) { ... ; return; }
--
Alexander E. Patrakov
_______________________________________________
pulseaudio-discuss mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
