On 2014-06-27 00:51:02 +0200, Petr Jelinek wrote:
> -     while ((c = getopt(argc, argv, "fl:m:no:O:x:e:")) != -1)
> +     while ((c = getopt(argc, argv, "fl:m:no:O:x:e:s::")) != -1)

Why two :?

>       {
>               switch (c)
>               {
> @@ -227,6 +229,33 @@ main(int argc, char *argv[])
>                               XLogFromFileName(optarg, &minXlogTli, 
> &minXlogSegNo);
>                               break;
>  
> +                     case 's':
> +                             if (optarg)
> +                             {
> +#ifdef HAVE_STRTOULL
> +                                     set_sysid = strtoull(optarg, &endptr, 
> 10);
> +#else
> +                                     set_sysid = strtoul(optarg, &endptr, 
> 10);
> +#endif

Wouldn't it be better to use sscanf()? That should work with large
inputs across platforms.

> +                                     /*
> +                                      * Validate input, we use strspn 
> because strtoul(l) accepts
> +                                      * negative numbers and silently 
> converts them to unsigned
> +                                      */
> +                                     if (set_sysid == 0 || errno != 0 ||
> +                                             endptr == optarg || *endptr != 
> '\0' ||
> +                                             strspn(optarg, "0123456789") != 
> strlen(optarg))
> +                                     {
> +                                             fprintf(stderr, _("%s: invalid 
> argument for option -s\n"), progname);
> +                                             fprintf(stderr, _("Try \"%s 
> --help\" for more information.\n"), progname);
> +                                             exit(1);
> +                                     }

Maybe: 'invalid argument \"%s\" ...'?

> @@ -1087,6 +1147,7 @@ usage(void)
>       printf(_("  -o OID           set next OID\n"));
>       printf(_("  -O OFFSET        set next multitransaction offset\n"));
>       printf(_("  -V, --version    output version information, then exit\n"));
> +     printf(_("  -s [SYSID]       set system identifier (or generate 
> one)\n"));
>       printf(_("  -x XID           set next transaction ID\n"));
>       printf(_("  -?, --help       show this help, then exit\n"));
>       printf(_("\nReport bugs to <pgsql-b...@postgresql.org>.\n"));

I think we usually try to keep the options roughly alphabetically
ordered. Same in the getopt() above.

Greetings,

Andres Freund


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to