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


-- 
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