Re: Set new system identifier using pg_resetxlog

From: Petr Jelinek <petr(at)2ndquadrant(dot)com>
To: Andres Freund <andres(at)2ndquadrant(dot)com>
Cc: Sawada Masahiko <sawada(dot)mshk(at)gmail(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Set new system identifier using pg_resetxlog
Date: 2014-07-18 11:08:24
Message-ID: 53C90028.8040901@2ndquadrant.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 18/07/14 00:41, Andres Freund wrote:
> On 2014-06-27 00:51:02 +0200, Petr Jelinek wrote:
>> {
>> 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.
>

Well, sscanf does not do much validation and silently truncates too big
input (which is why I replaced it with strtoull, original patch did have
sscanf), also I think the portability of sscanf might not be as good,
see 9d7ded0f4277f5c0063eca8e871a34e2355a8371 commit.

>> + /*
>> + * 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\" ...'?
>

Ok

>> @@ -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-bugs(at)postgresql(dot)org>.\n"));
>
> I think we usually try to keep the options roughly alphabetically
> ordered. Same in the getopt() above.
>

Ok, attached v4 with those changes.

--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

Attachment Content-Type Size
pg_resetxlog-sysid-v4.patch text/x-diff 5.9 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2014-07-18 11:18:55 Re: Set new system identifier using pg_resetxlog
Previous Message Magnus Hagander 2014-07-18 08:28:58 Re: [bug fix] pg_ctl always uses the same event source