Re: Set new system identifier using pg_resetxlog

On 2014-07-18 13:08:24 +0200, Petr Jelinek wrote:
> 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)

Well, the checks on length you've added should catch that when adapted
properly.

>, also I think the portability of sscanf might not be as good, see
> 9d7ded0f4277f5c0063eca8e871a34e2355a8371 commit.

Fair point. But I don't think the arguments why using strtoul instead of
strtoull is safe hold much water here. In the pg_stat_statement case the
worst that could happen is that the rowcount isn't accurate. Here it's
setting a wrong system identifier. Not really the same.

Maybe it's time to pursue
http://archives.postgresql.org/message-id/20140603144654.GM24145%40awork2.anarazel.de
further :(

Greetings,

Andres Freund