client: combine client_open() and client_parse_config()

Both users (client.c and audiod.c) called client_open() right
after calling client_parse_config(), so call client_open() from
client_parse_config() and make client_open() static.

While we're at it, rename client_open() to client_connect() as this
is what the function does. Also rename client_parse_config() to
client_open() and change audiod.c and client.c to call client_open()
instead of client_parse_config().

This patch fixes a segfault in para_client was it was run without
specifying a command. In this case, the private client data struct
was used by para_log after being freed -- Ouch. Fortunately, this
bug didn't affect para_audiod as it never calls these functions with
no command.

Another bug due to a typo in client_close() is also fixed. This code
needs closer investigation and lacks documentation btw.

diff --git a/audiod.c b/audiod.c
index 9009123ad6772b9c34c09269b3be3457fde3dfd6..2798ad26d57387c65779d9435b5ac471b901a54d 100644 (file)
--- a/audiod.c
+++ b/audiod.c
@@ -1019,17 +1019,14 @@ static void status_pre_select(struct sched *s, struct task *t)
                if (tv_diff(now, &st->clock_diff_barrier, NULL) < 0)
                        return;
                PARA_INFO_LOG("clock diff count: %d\n", st->clock_diff_count);
-               t->ret = client_parse_config(argc, argv, &st->pcd);
+               t->ret = client_open(argc, argv, &st->pcd);
 
        } else {
                char *argv[] = {"audiod", "stat", NULL};
                int argc = 2;
-               t->ret = client_parse_config(argc, argv, &st->pcd);
+               t->ret = client_open(argc, argv, &st->pcd);
        }
 
-       if (t->ret < 0)
-               return;
-       t->ret = client_open(st->pcd);
        if (t->ret < 0)
                return;
        st->pcd->task.event_handler = client_task_event_handler;

diff --git a/client.c b/client.c
index be76a491b3e1db0ce15efd194611cde12f719ccf..12812c9f79ecd0a4b5b88707c88572f709ea3fdd 100644 (file)
--- a/client.c
+++ b/client.c
@@ -74,17 +74,13 @@ int main(int argc, char *argv[])
 
        s.default_timeout.tv_sec = 1;
        s.default_timeout.tv_usec = 0;
-       ret = client_parse_config(argc, argv, &pcd);
-       if (ret < 0)
-               goto out;
+       ret = client_open(argc, argv, &pcd);
+       if (ret < 0) /* can not use PARA_LOG here */
+               exit(EXIT_FAILURE);
        pcd->task.event_handler = client_event_handler;
-       ret = client_open(pcd);
-       if (ret < 0)
-               goto out;
        ret = sched(&s);
-       client_close(pcd);
-out:
        if (ret < 0)
                PARA_ERROR_LOG("%s\n", PARA_STRERROR(-ret));
+       client_close(pcd);
        return ret >= 0? EXIT_SUCCESS: EXIT_FAILURE;
 }

diff --git a/client.h b/client.h
index 1c9a8c22ae4dabd73a20547c8ce6bc195989e218..ca50ee05da1c4d0fde86d93f9222ad7d37c0a704 100644 (file)
--- a/client.h
+++ b/client.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 1997-2006 Andre Noll <maan@systemlinux.org>
+ * Copyright (C) 1997-2007 Andre Noll <maan@systemlinux.org>
  *
  *     This program is free software; you can redistribute it and/or modify
  *     it under the terms of the GNU General Public License as published by
@@ -88,9 +88,7 @@ struct private_client_data {
        int *in_eof;
 };
 
-int client_open(struct private_client_data *pcd);
 void client_close(struct private_client_data *pcd);
-int client_parse_config(int argc, char *argv[],
-       struct private_client_data **pcd_ptr);
+int client_open(int argc, char *argv[], struct private_client_data **pcd_ptr);
 void client_pre_select(struct sched *s, struct task *t);
 void client_post_select(struct sched *s, struct task *t);

diff --git a/client_common.c b/client_common.c
index d13892599cd4b503c50164f9a7010ab40c6d2016..12c8365661e6c8f7e0608c66cd7d65a659e219cb 100644 (file)
--- a/client_common.c
+++ b/client_common.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 1997-2006 Andre Noll <maan@systemlinux.org>
+ * Copyright (C) 1997-2007 Andre Noll <maan@systemlinux.org>
  *
  *     This program is free software; you can redistribute it and/or modify
  *     it under the terms of the GNU General Public License as published by
@@ -48,7 +48,7 @@ void rc4_recv(unsigned long len, const unsigned char *indata,
 
 void client_close(struct private_client_data *pcd)
 {
-       if (pcd)
+       if (!pcd)
                return;
        if (pcd->fd >= 0) {
                disable_crypt(pcd->fd);
@@ -60,8 +60,43 @@ void client_close(struct private_client_data *pcd)
        free(pcd);
 }
 
-int client_parse_config(int argc, char *argv[],
-               struct private_client_data **pcd_ptr)
+static int client_connect(struct private_client_data *pcd)
+{
+       int ret;
+       struct hostent *he;
+       struct sockaddr_in their_addr;
+
+       pcd->fd = -1;
+       ret = get_host_info(pcd->conf.hostname_arg, &he);
+       if (ret < 0)
+               return ret;
+       /* get new socket */
+       ret = get_socket();
+       if (ret < 0)
+               return ret;
+       pcd->fd = ret;
+       /* init their_addr */
+       init_sockaddr(&their_addr, pcd->conf.server_port_arg, he);
+       ret = para_connect(pcd->fd, &their_addr);
+       if (ret < 0)
+               goto err_out;
+       pcd->status = CL_CONNECTED;
+       ret = mark_fd_nonblock(pcd->fd);
+       if (ret < 0)
+               goto err_out;
+       pcd->task.pre_select = client_pre_select;
+       pcd->task.post_select = client_post_select;
+       pcd->task.private_data = pcd;
+       sprintf(pcd->task.status, "client");
+       register_task(&pcd->task);
+       return 1;
+err_out:
+       close(pcd->fd);
+       pcd->fd = -1;
+       return ret;
+}
+
+int client_open(int argc, char *argv[], struct private_client_data **pcd_ptr)
 {
        char *home = para_homedir();
        struct stat statbuf;
@@ -69,9 +104,10 @@ int client_parse_config(int argc, char *argv[],
        struct private_client_data *pcd =
                para_calloc(sizeof(struct private_client_data));
 
+       *pcd_ptr = pcd;
        pcd->fd = -1;
-       client_cmdline_parser(argc, argv, &pcd->conf);
-       ret = - E_CLIENT_SYNTAX;
+       ret = client_cmdline_parser(argc, argv, &pcd->conf);
+       ret = -E_CLIENT_SYNTAX;
        if (!pcd->conf.inputs_num)
                goto out;
        pcd->user = pcd->conf.user_given?
@@ -93,16 +129,18 @@ int client_parse_config(int argc, char *argv[],
                client_cmdline_parser_configfile(pcd->config_file,
                        &pcd->conf, 0, 0, 0);
        ret = 1;
-       *pcd_ptr = pcd;
        PARA_INFO_LOG("loglevel: %d\n", pcd->conf.loglevel_arg);
        PARA_INFO_LOG("config_file: %s\n", pcd->config_file);
        PARA_INFO_LOG("key_file: %s\n", pcd->key_file);
        PARA_NOTICE_LOG("connecting %s:%d\n", pcd->conf.hostname_arg,
                pcd->conf.server_port_arg);
+       ret = client_connect(pcd);
 out:
        free(home);
-       if (ret < 0)
+       if (ret < 0) {
+               PARA_ERROR_LOG("%s\n", PARA_STRERROR(-ret));
                client_close(pcd);
+       }
        return ret;
 }
 
@@ -288,39 +326,3 @@ void client_post_select(struct sched *s, struct task *t)
        }
 
 }
-
-int client_open(struct private_client_data *pcd)
-{
-       int ret;
-       struct hostent *he;
-       struct sockaddr_in their_addr;
-
-       pcd->fd = -1;
-       ret = get_host_info(pcd->conf.hostname_arg, &he);
-       if (ret < 0)
-               goto err_out;
-       /* get new socket */
-       ret = get_socket();
-       if (ret < 0)
-               goto err_out;
-       pcd->fd = ret;
-       /* init their_addr */
-       init_sockaddr(&their_addr, pcd->conf.server_port_arg, he);
-       ret = para_connect(pcd->fd, &their_addr);
-       if (ret < 0)
-               goto err_out;
-       pcd->status = CL_CONNECTED;
-       ret = mark_fd_nonblock(pcd->fd);
-       if (ret < 0)
-               goto err_out;
-       pcd->task.pre_select = client_pre_select;
-       pcd->task.post_select = client_post_select;
-       pcd->task.private_data = pcd;
-       sprintf(pcd->task.status, "client");
-       register_task(&pcd->task);
-       return 1;
-err_out:
-       if (pcd->fd >= 0)
-               close(pcd->fd);
-       return ret;
-}