| Message ID | 1435751267-26378-7-git-send-email-marcandre.lureau@gmail.com |
|---|---|
| State | New |
| Headers | show |
Quoting Marc-André Lureau (2015-07-01 06:47:41) > Move option parsing out of giant main(). > > Signed-off-by: Marc-André Lureau <marcandre.lureau@gmail.com> > --- > qga/main.c | 41 +++++++++++++++++++++++++---------------- > 1 file changed, 25 insertions(+), 16 deletions(-) > > diff --git a/qga/main.c b/qga/main.c > index b776d16..b965f61 100644 > --- a/qga/main.c > +++ b/qga/main.c > @@ -941,19 +941,25 @@ static GList *split_list(gchar *str, const gchar separator) > return list; > } > > -int main(int argc, char **argv) > -{ > - const char *sopt = "hVvdm:p:l:f:F::b:s:t:"; > - char *method = NULL, *device_path = NULL; > - char *log_filepath = NULL; > - char *pid_filepath = NULL; > +static char *device_path; > +static char *method; > +static char *log_filepath; > +static char *pid_filepath; Since we want to pass these around as a representation of the configuration state, I'd rather we package them into a GAConfig structure or something of the sort that and pass it around as arguments rather than as globals. Between parse/load_config/load_defaults it's becoming a little difficult to keep track of where all these values are being modified. Otherwise, looks good, and makes for a nice cleanup. > #ifdef CONFIG_FSFREEZE > - char *fsfreeze_hook = NULL; > +static char *fsfreeze_hook; > #endif > - char *state_dir = NULL; > +static char *state_dir; > #ifdef _WIN32 > - const char *service = NULL; > +static const char *service; > #endif > +static GList *blacklist; > +static int daemonize; > +static GLogLevelFlags log_level = G_LOG_LEVEL_ERROR | G_LOG_LEVEL_CRITICAL; > + > +static void option_parse(int argc, char **argv) > +{ > + const char *sopt = "hVvdm:p:l:f:F::b:s:t:D"; > + int opt_ind = 0, ch; > const struct option lopt[] = { > { "help", 0, NULL, 'h' }, > { "version", 0, NULL, 'V' }, > @@ -973,14 +979,7 @@ int main(int argc, char **argv) > { "statedir", 1, NULL, 't' }, > { NULL, 0, NULL, 0 } > }; > - int opt_ind = 0, ch, daemonize = 0; > - GLogLevelFlags log_level = G_LOG_LEVEL_ERROR | G_LOG_LEVEL_CRITICAL; > - GList *blacklist = NULL; > - GAState *s; > > - module_call_init(MODULE_INIT_QAPI); > - > - init_dfl_pathnames(); > while ((ch = getopt_long(argc, argv, sopt, lopt, &opt_ind)) != -1) { > switch (ch) { > case 'm': > @@ -1058,6 +1057,16 @@ int main(int argc, char **argv) > exit(EXIT_FAILURE); > } > } > +} > + > +int main(int argc, char **argv) > +{ > + GAState *s; > + > + module_call_init(MODULE_INIT_QAPI); > + > + init_dfl_pathnames(); > + option_parse(argc, argv); > > if (pid_filepath == NULL) { > pid_filepath = g_strdup(dfl_pathnames.pidfile); > -- > 2.4.3 >
hi On Tue, Aug 25, 2015 at 6:24 PM, Michael Roth <mdroth@linux.vnet.ibm.com> wrote: > Since we want to pass these around as a representation of the > configuration state, I'd rather we package them into a GAConfig > structure or something of the sort that and pass it around as arguments > rather than as globals. Between parse/load_config/load_defaults it's > becoming a little difficult to keep track of where all these values are > being modified. done, thanks
diff --git a/qga/main.c b/qga/main.c index b776d16..b965f61 100644 --- a/qga/main.c +++ b/qga/main.c @@ -941,19 +941,25 @@ static GList *split_list(gchar *str, const gchar separator) return list; } -int main(int argc, char **argv) -{ - const char *sopt = "hVvdm:p:l:f:F::b:s:t:"; - char *method = NULL, *device_path = NULL; - char *log_filepath = NULL; - char *pid_filepath = NULL; +static char *device_path; +static char *method; +static char *log_filepath; +static char *pid_filepath; #ifdef CONFIG_FSFREEZE - char *fsfreeze_hook = NULL; +static char *fsfreeze_hook; #endif - char *state_dir = NULL; +static char *state_dir; #ifdef _WIN32 - const char *service = NULL; +static const char *service; #endif +static GList *blacklist; +static int daemonize; +static GLogLevelFlags log_level = G_LOG_LEVEL_ERROR | G_LOG_LEVEL_CRITICAL; + +static void option_parse(int argc, char **argv) +{ + const char *sopt = "hVvdm:p:l:f:F::b:s:t:D"; + int opt_ind = 0, ch; const struct option lopt[] = { { "help", 0, NULL, 'h' }, { "version", 0, NULL, 'V' }, @@ -973,14 +979,7 @@ int main(int argc, char **argv) { "statedir", 1, NULL, 't' }, { NULL, 0, NULL, 0 } }; - int opt_ind = 0, ch, daemonize = 0; - GLogLevelFlags log_level = G_LOG_LEVEL_ERROR | G_LOG_LEVEL_CRITICAL; - GList *blacklist = NULL; - GAState *s; - module_call_init(MODULE_INIT_QAPI); - - init_dfl_pathnames(); while ((ch = getopt_long(argc, argv, sopt, lopt, &opt_ind)) != -1) { switch (ch) { case 'm': @@ -1058,6 +1057,16 @@ int main(int argc, char **argv) exit(EXIT_FAILURE); } } +} + +int main(int argc, char **argv) +{ + GAState *s; + + module_call_init(MODULE_INIT_QAPI); + + init_dfl_pathnames(); + option_parse(argc, argv); if (pid_filepath == NULL) { pid_filepath = g_strdup(dfl_pathnames.pidfile);
Move option parsing out of giant main(). Signed-off-by: Marc-André Lureau <marcandre.lureau@gmail.com> --- qga/main.c | 41 +++++++++++++++++++++++++---------------- 1 file changed, 25 insertions(+), 16 deletions(-)