Re: Check for integer overflow in datetime functions

Neil Conway <neilc(at)samurai(dot)com> writes:
> It seems a bit laborious to always manually set errno to zero before
> invoking strtol() (both in the places added by the patch, and all the
> places that already did that). While it's only a minor notational
> improvement, I wonder if it would be worth adding a pg_strtol() that did
> the errno assignment so that each call-site wouldn't need to worry about
> it?

Don't see the point really, since the check of errno would still have to
be in the calling code (since pg_strtol wouldn't know what the
appropriate error action is). So the choice is between

errno = 0;
foo = strtol(...);
if (errno == ERANGE)
... something ...

and

foo = pg_strtol(...);
if (errno == ERANGE)
... something ...

I don't think there's less cognitive load in the second case,
particularly not to someone who's familiar with the standard behavior
of strtol() --- you'll be constantly thinking "what if errno is already
ERANGE ... oh, pg_strtol resets it ..."

I'd be in favor of encapsulating the whole sequence including an
ereport(ERROR) into one function, except it seems we'd not use it in
very many places.

BTW, Neil, were you looking at this with intention to commit it?
I'll pick it up if not, but don't want to duplicate effort if you've
already started on it.

regards, tom lane