[ovs-dev] [ovsdb-locks 05/11] ovs-vswitchd: Make database socket command-line argument optional.
Ethan Jackson
ethan at nicira.com
Tue Jul 26 18:36:47 UTC 2011
Looks good to me.
Ethan
On Tue, Jul 26, 2011 at 10:13, Ben Pfaff <blp at nicira.com> wrote:
> On Mon, Jul 25, 2011 at 02:51:54PM -0700, Ethan Jackson wrote:
>> Nifty! I like this patch.
>>
>> > - ? ?if (argc != 1) {
>> > + ? ?switch (argc) {
>> > + ? ?case 0:
>> > + ? ? ? ?return xasprintf("unix:%s/db.sock", ovs_rundir());
>> > +
>> > + ? ?case 1:
>> > + ? ? ? ?return argv[0];
>> > +
>> > + ? ?default:
>> > ? ? ? ? VLOG_FATAL("database socket is only non-option argument; "
>> > ? ? ? ? ? ? ? ? ? ?"use --help for usage");
>> > ? ? }
>>
>> I'll make the same comment here that I did in the previous patch about
>> xstrdup-ing argv[0].
>
> I made the same change here, thanks.
>
>> Also I think the VLOG_FATAL message needs to be updated.
>
> I think it's still technically correct, but I changed it to "at most one
> non-option argument accepted". Is that more helpful?
>
> I noticed that the --help output needed an update.
>
> Here's an incremental. I won't wait for re-review, but please do point
> out mistakes for me to fix later.
>
> diff --git a/vswitchd/ovs-vswitchd.c b/vswitchd/ovs-vswitchd.c
> index 07dcd28..7d4e4d7 100644
> --- a/vswitchd/ovs-vswitchd.c
> +++ b/vswitchd/ovs-vswitchd.c
> @@ -53,7 +53,7 @@ VLOG_DEFINE_THIS_MODULE(vswitchd);
>
> static unixctl_cb_func ovs_vswitchd_exit;
>
> -static const char *parse_options(int argc, char *argv[]);
> +static char *parse_options(int argc, char *argv[]);
> static void usage(void) NO_RETURN;
>
> int
> @@ -61,7 +61,7 @@ main(int argc, char *argv[])
> {
> struct unixctl_server *unixctl;
> struct signal *sighup;
> - const char *remote;
> + char *remote;
> bool exiting;
> int retval;
>
> @@ -83,6 +83,8 @@ main(int argc, char *argv[])
> unixctl_command_register("exit", ovs_vswitchd_exit, &exiting);
>
> bridge_init(remote);
> + free(remote);
> +
> exiting = false;
> while (!exiting) {
> if (signal_poll(sighup)) {
> @@ -108,7 +110,7 @@ main(int argc, char *argv[])
> return 0;
> }
>
> -static const char *
> +static char *
> parse_options(int argc, char *argv[])
> {
> enum {
> @@ -196,10 +198,10 @@ parse_options(int argc, char *argv[])
> return xasprintf("unix:%s/db.sock", ovs_rundir());
>
> case 1:
> - return argv[0];
> + return xstrdup(argv[0]);
>
> default:
> - VLOG_FATAL("database socket is only non-option argument; "
> + VLOG_FATAL("at most one non-option argument accepted; "
> "use --help for usage");
> }
> }
> @@ -208,9 +210,10 @@ static void
> usage(void)
> {
> printf("%s: Open vSwitch daemon\n"
> - "usage: %s [OPTIONS] DATABASE\n"
> - "where DATABASE is a socket on which ovsdb-server is listening.\n",
> - program_name, program_name);
> + "usage: %s [OPTIONS] [DATABASE]\n"
> + "where DATABASE is a socket on which ovsdb-server is listening\n"
> + " (default: \"unix:%s/db.sock\").\n",
> + program_name, program_name, ovs_rundir());
> stream_usage("DATABASE", true, false, true);
> daemon_usage();
> vlog_usage();
>
More information about the dev
mailing list