Re: [PATCHES] Check for integer overflow in datetime functions

2005-12-02 Thread Joshua D. Drake 




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

2005-12-02 Thread Andrew Dunstan 



Magnus Hagander wrote:

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



Yes, and I for one love it :-)

Now if I could just access the actual svn repository as well, I'd be
even happier...


 



I have added it to the set of useful links on pgfoundry's front page.

cheers

andrew

---(end of broadcast)---
TIP 6: explain analyze is your friend

Re: [PATCHES] Check for integer overflow in datetime functions

2005-12-01 Thread Christopher Kings-Lynne 

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


Does anyone else find the PostgreSQL "TRUNK" funny? :D


---(end of broadcast)---
TIP 3: Have you checked our extensive FAQ?

  http://www.postgresql.org/docs/faq

Re: [PATCHES] Check for integer overflow in datetime functions

2005-12-01 Thread Magnus Hagander 
> 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

Re: [PATCHES] Check for integer overflow in datetime functions

2005-12-01 Thread Tom Lane 
Michael Fuhr <[EMAIL PROTECTED]> writes:
> Check integer conversion for overflow in datetime functions.

Applied back to 7.4.  The patch would not work in 7.3 because we didn't
have the DTERR_ convention back then, and it seems not worth the effort
to try to deal with the problem that far back.

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

I left it as-is ...

regards, tom lane

---(end of broadcast)---
TIP 1: if posting/reading through Usenet, please send an appropriate
   subscribe-nomail command to [EMAIL PROTECTED] so that your
   message can get through to the mailing list cleanly

Re: [PATCHES] Check for integer overflow in datetime functions

2005-12-01 Thread Tom Lane 
Alvaro Herrera <[EMAIL PROTECTED]> writes:
> 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?

Well, they're not *exactly* simultaneous of course.  I set up the
modified files in each tree and then commit, commit, commit (the -F
option helps if the commit message is long).  I use cvs2cl to extract
CVS history, and it will fold together commits in different branches
if they have the same commit message and are within some time delta
of each other ... I think it's 5 minutes or so.

regards, tom lane

---(end of broadcast)---
TIP 6: explain analyze is your friend

Re: [PATCHES] Check for integer overflow in datetime functions

2005-12-01 Thread Alvaro Herrera 
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

2005-12-01 Thread Tom Lane 
Michael Fuhr <[EMAIL PROTECTED]> writes:
> Check integer conversion for overflow in datetime functions.

Looks reasonable, will check and apply.

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

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

regards, tom lane

---(end of broadcast)---
TIP 3: Have you checked our extensive FAQ?

   http://www.postgresql.org/docs/faq

Re: [PATCHES] Check for integer overflow in datetime functions

2005-12-01 Thread Tom Lane 
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

2005-12-01 Thread Neil Conway 
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

[PATCHES] Check for integer overflow in datetime functions

2005-11-30 Thread Michael Fuhr 
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)
+