From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48187) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZUMzJ-0001q7-Uc for qemu-devel@nongnu.org; Tue, 25 Aug 2015 18:46:43 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZUMzG-0007CD-Lf for qemu-devel@nongnu.org; Tue, 25 Aug 2015 18:46:41 -0400 Received: from e31.co.us.ibm.com ([32.97.110.149]:48906) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZUMzG-0007C3-Ew for qemu-devel@nongnu.org; Tue, 25 Aug 2015 18:46:38 -0400 Received: from /spool/local by e31.co.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Tue, 25 Aug 2015 16:46:37 -0600 Received: from b03cxnp07029.gho.boulder.ibm.com (b03cxnp07029.gho.boulder.ibm.com [9.17.130.16]) by d03dlp03.boulder.ibm.com (Postfix) with ESMTP id 130C519D803F for ; Tue, 25 Aug 2015 16:37:30 -0600 (MDT) Received: from d03av01.boulder.ibm.com (d03av01.boulder.ibm.com [9.17.195.167]) by b03cxnp07029.gho.boulder.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id t7PMkYHx54984754 for ; Tue, 25 Aug 2015 15:46:34 -0700 Received: from d03av01.boulder.ibm.com (localhost [127.0.0.1]) by d03av01.boulder.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id t7PMkX4e010338 for ; Tue, 25 Aug 2015 16:46:33 -0600 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable From: Michael Roth In-Reply-To: <1440540624-7998-7-git-send-email-marcandre.lureau@redhat.com> References: <1440540624-7998-1-git-send-email-marcandre.lureau@redhat.com> <1440540624-7998-7-git-send-email-marcandre.lureau@redhat.com> Message-ID: <20150825224044.11069.6653@loki> Date: Tue, 25 Aug 2015 17:40:44 -0500 Subject: Re: [Qemu-devel] [PATCH v2 06/12] qga: move option parsing to seperate function List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: marcandre.lureau@redhat.com, qemu-devel@nongnu.org Quoting marcandre.lureau@redhat.com (2015-08-25 17:10:18) > From: Marc-Andr=C3=A9 Lureau > = > Move option parsing out of giant main(). > = > Signed-off-by: Marc-Andr=C3=A9 Lureau Reviewed-by: Michael Roth > --- > qga/main.c | 165 +++++++++++++++++++++++++++++++++++--------------------= ------ > 1 file changed, 96 insertions(+), 69 deletions(-) > = > diff --git a/qga/main.c b/qga/main.c > index 83b7804..a8dda38 100644 > --- a/qga/main.c > +++ b/qga/main.c > @@ -941,19 +941,28 @@ static GList *split_list(gchar *str, const gchar se= parator) > return list; > } > = > -int main(int argc, char **argv) > -{ > - const char *sopt =3D "hVvdm:p:l:f:F::b:s:t:"; > - char *method =3D NULL, *channel_path =3D NULL; > - char *log_filepath =3D NULL; > - char *pid_filepath =3D NULL; > +typedef struct GAConfig { > + char *channel_path; > + char *method; > + char *log_filepath; > + char *pid_filepath; > #ifdef CONFIG_FSFREEZE > - char *fsfreeze_hook =3D NULL; > + char *fsfreeze_hook; > #endif > - char *state_dir =3D NULL; > + char *state_dir; > #ifdef _WIN32 > - const char *service =3D NULL; > + const char *service; > #endif > + GList *blacklist; > + int daemonize; > + GLogLevelFlags log_level; > +} GAConfig; > + > +static GAConfig *config_parse(int argc, char **argv) > +{ > + GAConfig *config =3D g_new0(GAConfig, 1); > + const char *sopt =3D "hVvdm:p:l:f:F::b:s:t:D"; > + int opt_ind =3D 0, ch; > const struct option lopt[] =3D { > { "help", 0, NULL, 'h' }, > { "version", 0, NULL, 'V' }, > @@ -973,74 +982,71 @@ int main(int argc, char **argv) > { "statedir", 1, NULL, 't' }, > { NULL, 0, NULL, 0 } > }; > - int opt_ind =3D 0, ch, daemonize =3D 0; > - GLogLevelFlags log_level =3D G_LOG_LEVEL_ERROR | G_LOG_LEVEL_CRITICA= L; > - GList *blacklist =3D NULL; > - GAState *s; > = > - module_call_init(MODULE_INIT_QAPI); > + config->log_level =3D G_LOG_LEVEL_ERROR | G_LOG_LEVEL_CRITICAL; > = > - init_dfl_pathnames(); > while ((ch =3D getopt_long(argc, argv, sopt, lopt, &opt_ind)) !=3D -= 1) { > switch (ch) { > case 'm': > - method =3D g_strdup(optarg); > + config->method =3D g_strdup(optarg); > break; > case 'p': > - channel_path =3D g_strdup(optarg); > + config->channel_path =3D g_strdup(optarg); > break; > case 'l': > - log_filepath =3D g_strdup(optarg); > + config->log_filepath =3D g_strdup(optarg); > break; > case 'f': > - pid_filepath =3D g_strdup(optarg); > + config->pid_filepath =3D g_strdup(optarg); > break; > #ifdef CONFIG_FSFREEZE > case 'F': > - fsfreeze_hook =3D g_strdup(optarg ?: QGA_FSFREEZE_HOOK_DEFAU= LT); > + config->fsfreeze_hook =3D g_strdup(optarg ?: QGA_FSFREEZE_HO= OK_DEFAULT); > break; > #endif > case 't': > - state_dir =3D g_strdup(optarg); > + config->state_dir =3D g_strdup(optarg); > break; > case 'v': > /* enable all log levels */ > - log_level =3D G_LOG_LEVEL_MASK; > + config->log_level =3D G_LOG_LEVEL_MASK; > break; > case 'V': > printf("QEMU Guest Agent %s\n", QEMU_VERSION); > exit(EXIT_SUCCESS); > case 'd': > - daemonize =3D 1; > + config->daemonize =3D 1; > break; > case 'b': { > if (is_help_option(optarg)) { > qmp_for_each_command(ga_print_cmd, NULL); > exit(EXIT_SUCCESS); > } > - blacklist =3D g_list_concat(blacklist, split_list(optarg, ',= ')); > + config->blacklist =3D g_list_concat(config->blacklist, > + split_list(optarg, ',')); > break; > } > #ifdef _WIN32 > case 's': > - service =3D optarg; > - if (strcmp(service, "install") =3D=3D 0) { > + config->service =3D optarg; > + if (strcmp(config->service, "install") =3D=3D 0) { > if (ga_install_vss_provider()) { > exit(EXIT_FAILURE); > } > - if (ga_install_service(channel_path, log_filepath, state= _dir)) { > + if (ga_install_service(config->channel_path, > + config->log_filepath, config->sta= te_dir)) { > exit(EXIT_FAILURE); > } > exit(EXIT_SUCCESS); > - } else if (strcmp(service, "uninstall") =3D=3D 0) { > + } else if (strcmp(config->service, "uninstall") =3D=3D 0) { > ga_uninstall_vss_provider(); > exit(ga_uninstall_service()); > - } else if (strcmp(service, "vss-install") =3D=3D 0) { > + } else if (strcmp(config->service, "vss-install") =3D=3D 0) { > if (ga_install_vss_provider()) { > exit(EXIT_FAILURE); > } > exit(EXIT_SUCCESS); > - } else if (strcmp(service, "vss-uninstall") =3D=3D 0) { > + } else if (strcmp(config->service, "vss-uninstall") =3D=3D 0= ) { > ga_uninstall_vss_provider(); > exit(EXIT_SUCCESS); > } else { > @@ -1059,12 +1065,39 @@ int main(int argc, char **argv) > } > } > = > - if (pid_filepath =3D=3D NULL) { > - pid_filepath =3D g_strdup(dfl_pathnames.pidfile); > + return config; > +} > + > +static void config_free(GAConfig *config) > +{ > + g_free(config->method); > + g_free(config->log_filepath); > + g_free(config->pid_filepath); > + g_free(config->state_dir); > + g_free(config->channel_path); > +#ifdef CONFIG_FSFREEZE > + g_free(config->fsfreeze_hook); > +#endif > + g_free(config); > +} > + > + int main(int argc, char **argv) > +{ > + GAState *s; > + GAConfig *config; > + > + module_call_init(MODULE_INIT_QAPI); > + > + init_dfl_pathnames(); > + > + config =3D config_parse(argc, argv); > + > + if (config->pid_filepath =3D=3D NULL) { > + config->pid_filepath =3D g_strdup(dfl_pathnames.pidfile); > } > = > - if (state_dir =3D=3D NULL) { > - state_dir =3D g_strdup(dfl_pathnames.state_dir); > + if (config->state_dir =3D=3D NULL) { > + config->state_dir =3D g_strdup(dfl_pathnames.state_dir); > } > = > #ifdef _WIN32 > @@ -1074,25 +1107,25 @@ int main(int argc, char **argv) > * error later on, we won't try to clean up the directory, it is con= sidered > * persistent. > */ > - if (g_mkdir_with_parents(state_dir, S_IRWXU) =3D=3D -1) { > + if (g_mkdir_with_parents(config->state_dir, S_IRWXU) =3D=3D -1) { > g_critical("unable to create (an ancestor of) the state director= y" > - " '%s': %s", state_dir, strerror(errno)); > + " '%s': %s", config->state_dir, strerror(errno)); > return EXIT_FAILURE; > } > #endif > = > s =3D g_malloc0(sizeof(GAState)); > - s->log_level =3D log_level; > + s->log_level =3D config->log_level; > s->log_file =3D stderr; > #ifdef CONFIG_FSFREEZE > - s->fsfreeze_hook =3D fsfreeze_hook; > + s->fsfreeze_hook =3D config->fsfreeze_hook; > #endif > g_log_set_default_handler(ga_log, s); > g_log_set_fatal_mask(NULL, G_LOG_LEVEL_ERROR); > ga_enable_logging(s); > s->state_filepath_isfrozen =3D g_strdup_printf("%s/qga.state.isfroze= n", > - state_dir); > - s->pstate_filepath =3D g_strdup_printf("%s/qga.state", state_dir); > + config->state_dir); > + s->pstate_filepath =3D g_strdup_printf("%s/qga.state", config->state= _dir); > s->frozen =3D false; > = > #ifndef _WIN32 > @@ -1125,23 +1158,23 @@ int main(int argc, char **argv) > #endif > = > if (ga_is_frozen(s)) { > - if (daemonize) { > + if (config->daemonize) { > /* delay opening/locking of pidfile till filesystems are unf= rozen */ > - s->deferred_options.pid_filepath =3D pid_filepath; > + s->deferred_options.pid_filepath =3D config->pid_filepath; > become_daemon(NULL); > } > - if (log_filepath) { > + if (config->log_filepath) { > /* delay opening the log file till filesystems are unfrozen = */ > - s->deferred_options.log_filepath =3D log_filepath; > + s->deferred_options.log_filepath =3D config->log_filepath; > } > ga_disable_logging(s); > qmp_for_each_command(ga_disable_non_whitelisted, NULL); > } else { > - if (daemonize) { > - become_daemon(pid_filepath); > + if (config->daemonize) { > + become_daemon(config->pid_filepath); > } > - if (log_filepath) { > - FILE *log_file =3D ga_open_logfile(log_filepath); > + if (config->log_filepath) { > + FILE *log_file =3D ga_open_logfile(config->log_filepath); > if (!log_file) { > g_critical("unable to open specified log file: %s", > strerror(errno)); > @@ -1159,14 +1192,15 @@ int main(int argc, char **argv) > goto out_bad; > } > = > - blacklist =3D ga_command_blacklist_init(blacklist); > - if (blacklist) { > - s->blacklist =3D blacklist; > + config->blacklist =3D ga_command_blacklist_init(config->blacklist); > + if (config->blacklist) { > + GList *l =3D config->blacklist; > + s->blacklist =3D config->blacklist; > do { > - g_debug("disabling command: %s", (char *)blacklist->data); > - qmp_disable_command(blacklist->data); > - blacklist =3D g_list_next(blacklist); > - } while (blacklist); > + g_debug("disabling command: %s", (char *)l->data); > + qmp_disable_command(l->data); > + l =3D g_list_next(l); > + } while (l); > } > s->command_state =3D ga_command_state_new(); > ga_command_state_init(s, s->command_state); > @@ -1181,14 +1215,14 @@ int main(int argc, char **argv) > #endif > = > s->main_loop =3D g_main_loop_new(NULL, false); > - if (!channel_init(ga_state, method, channel_path)) { > + if (!channel_init(ga_state, config->method, config->channel_path)) { > g_critical("failed to initialize guest agent channel"); > goto out_bad; > } > #ifndef _WIN32 > g_main_loop_run(ga_state->main_loop); > #else > - if (daemonize) { > + if (config->daemonize) { > SERVICE_TABLE_ENTRY service_table[] =3D { > { (char *)QGA_SERVICE_NAME, service_main }, { NULL, NULL } }; > StartServiceCtrlDispatcher(service_table); > @@ -1200,24 +1234,17 @@ int main(int argc, char **argv) > ga_command_state_cleanup_all(ga_state->command_state); > ga_channel_free(ga_state->channel); > = > - if (daemonize) { > - unlink(pid_filepath); > + if (config->daemonize) { > + unlink(config->pid_filepath); > } > return 0; > = > out_bad: > - if (daemonize) { > - unlink(pid_filepath); > + if (config->daemonize) { > + unlink(config->pid_filepath); > } > = > - g_free(method); > - g_free(log_filepath); > - g_free(pid_filepath); > - g_free(state_dir); > - g_free(channel_path); > -#ifdef CONFIG_FSFREEZE > - g_free(fsfreeze_hook); > -#endif > + config_free(config); > = > return EXIT_FAILURE; > } > -- = > 2.4.3 >=20