Re: [PATCHES] Check for integer overflow in datetime functions
http://projects.commandprompt.com/projects/public/pgsql/browser/trunk/pgsql It has the additional advantage over our current CVSweb that it's set with tabs to 4 spaces, so it looks just like our code is supposed to ... I need to spend some time on it to see if there is a way that I can convert more then the whole tree at a time within the same repository. Right now we have to convert the whole thing which takes quite a bit of time. Oh and don't forget to use the RSS capabilities. Yes, and I for one love it :-) Now if I could just access the actual svn repository as well, I'd be even happier... We are also happy to provide accounts to those who would like to actually use the wiki to discuss specific aspects of the code. This would be more for wannabe hacker edibility, tips, tricks etc... I have added it to the set of useful links on pgfoundry's front page. Cool! Joshua D. Drake cheers andrew ---(end of broadcast)--- TIP 6: explain analyze is your friend -- The PostgreSQL Company - Command Prompt, Inc. 1.503.667.4564 PostgreSQL Replication, Consulting, Custom Development, 24x7 support Managed Services, Shared and Dedicated Hosting Co-Authors: PLphp, PLperl - http://www.commandprompt.com/ ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] Check for integer overflow in datetime functions
On Wed, 2005-11-30 at 19:36 -0700, Michael Fuhr wrote: Check integer conversion for overflow in datetime functions. 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? -Neil ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] Check for integer overflow in datetime functions
Neil Conway [EMAIL PROTECTED] 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 ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [PATCHES] Check for integer overflow in datetime functions
Tom Lane wrote: Don't worry about that, I'll take care of it. I prefer committing all the branches at once when doing a multi-branch fix (less clutter in the CVS logs). How do you do that? I have multiple checked-out trees, I assume you do the same and just handle the simultaneous-ness by hand? BTW, has anyone checked Command Prompt's Subversion repository? It's a mirror of our anonymous CVS (AFAICT). I'm using it for reading diffs lately, and it's much nicer to look at the whole patch as a single diff rather than going a single file at a time. http://projects.commandprompt.com/projects/public/pgsql/browser/trunk/pgsql It has the additional advantage over our current CVSweb that it's set with tabs to 4 spaces, so it looks just like our code is supposed to ... -- Alvaro Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] Check for integer overflow in datetime functions
BTW, has anyone checked Command Prompt's Subversion repository? It's a mirror of our anonymous CVS (AFAICT). I'm using it for reading diffs lately, and it's much nicer to look at the whole patch as a single diff rather than going a single file at a time. http://projects.commandprompt.com/projects/public/pgsql/browse r/trunk/pgsql It has the additional advantage over our current CVSweb that it's set with tabs to 4 spaces, so it looks just like our code is supposed to ... Yes, and I for one love it :-) Now if I could just access the actual svn repository as well, I'd be even happier... //Magnus ---(end of broadcast)--- TIP 6: explain analyze is your friend
[PATCHES] Check for integer overflow in datetime functions
Check integer conversion for overflow in datetime functions. Problem
reported by Christopher Kings-Lynne:
http://archives.postgresql.org/pgsql-hackers/2005-11/msg01385.php
In one place (line 60 in the patch) the code sets errno to 0 when
it should still be 0 after similar code a few lines above (otherwise
the function would have returned an error). I wasn't sure what
would be preferred: clearing errno again just to be sure, a comment
explaining why it isn't necessary, or nothing at all.
This patch should apply cleanly against HEAD and 8.1. If the patch
looks good then I'll submit patches for earlier branches when I get
a chance to build and test those versions.
--
Michael Fuhr
Index: src/backend/utils/adt/datetime.c
===
RCS file: /projects/cvsroot/pgsql/src/backend/utils/adt/datetime.c,v
retrieving revision 1.160.2.1
diff -c -r1.160.2.1 datetime.c
*** src/backend/utils/adt/datetime.c22 Nov 2005 18:23:21 -
1.160.2.1
--- src/backend/utils/adt/datetime.c1 Dec 2005 01:12:01 -
***
*** 1013,1019
--- 1013,1022
if (tzp == NULL)
return DTERR_BAD_FORMAT;
+ errno = 0;
val = strtol(field[i], cp, 10);
+ if (errno == ERANGE)
+ return DTERR_FIELD_OVERFLOW;
j2date(val, tm-tm_year, tm-tm_mon,
tm-tm_mday);
/* Get the time zone from the end of
the string */
***
*** 1158,1164
--- 1161,1170
char *cp;
int val;
+ errno = 0;
val = strtol(field[i], cp, 10);
+ if (errno == ERANGE)
+ return DTERR_FIELD_OVERFLOW;
/*
* only a few kinds are allowed to have
an embedded
***
*** 1915,1921
--- 1921,1930
break;
}
+ errno = 0;
val = strtol(field[i], cp, 10);
+ if (errno == ERANGE)
+ return DTERR_FIELD_OVERFLOW;
/*
* only a few kinds are allowed to have
an embedded
***
*** 2456,2466
--- 2465,2481
*tmask = DTK_TIME_M;
+ errno = 0;
tm-tm_hour = strtol(str, cp, 10);
+ if (errno == ERANGE)
+ return DTERR_FIELD_OVERFLOW;
if (*cp != ':')
return DTERR_BAD_FORMAT;
str = cp + 1;
+ errno = 0;
tm-tm_min = strtol(str, cp, 10);
+ if (errno == ERANGE)
+ return DTERR_FIELD_OVERFLOW;
if (*cp == '\0')
{
tm-tm_sec = 0;
***
*** 2471,2477
--- 2486,2495
else
{
str = cp + 1;
+ errno = 0;
tm-tm_sec = strtol(str, cp, 10);
+ if (errno == ERANGE)
+ return DTERR_FIELD_OVERFLOW;
if (*cp == '\0')
*fsec = 0;
else if (*cp == '.')
***
*** 2522,2528
--- 2540,2549
*tmask = 0;
+ errno = 0;
val = strtol(str, cp, 10);
+ if (errno == ERANGE)
+ return DTERR_FIELD_OVERFLOW;
if (cp == str)
return DTERR_BAD_FORMAT;
***
*** 2809,2819
--- 2830,2848
if (*str != '+' *str != '-')
return DTERR_BAD_FORMAT;
+ errno = 0;
hr = strtol(str + 1, cp, 10);
+ if (errno == ERANGE)
+ return DTERR_TZDISP_OVERFLOW;
/* explicit delimiter? */
if (*cp == ':')
+ {
+ errno = 0;
min = strtol(cp + 1, cp, 10);
+ if (errno == ERANGE)
+ return DTERR_TZDISP_OVERFLOW;
+ }
/* otherwise, might have run things together... */
else if (*cp == '\0' strlen(str) 3)
{
***
*** 3056,3062
--- 3085,3094
case DTK_DATE:
case DTK_NUMBER:
+ errno = 0;
val = strtol(field[i], cp, 10);
+ if (errno == ERANGE)
+ return
