[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