Skip site navigation (1) Skip section navigation (2)

Re: [PATCHES] to_date() validation

From: Martijn van Oosterhout <kleptog(at)svana(dot)org>
To: Brendan Jurd <direvus(at)gmail(dot)com>
Cc: Alex Hunsaker <badalex(at)gmail(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: [PATCHES] to_date() validation
Date: 2008-09-09 09:29:19
Message-ID: 20080909092919.GB29604@svana.org (view raw or flat)
Thread:
Lists: pgsql-hackerspgsql-patches
On Mon, Sep 08, 2008 at 06:24:14PM +1000, Brendan Jurd wrote:
> On Sun, Sep 7, 2008 at 5:58 AM, Alex Hunsaker <badalex(at)gmail(dot)com> wrote:
> > Im just following this:
> > http://wiki.postgresql.org/wiki/Reviewing_a_Patch so lets get started.
> >
> 
> Hi Alex.  Thanks for taking the time to review my patch.

I actually had a look at this patch also, though not as thoroughly as
Alex. There was one part that I had some thoughts about in from_char_parse_int_len:

!               /*
!                * We need to pull exactly the number of characters given in 'len' out
!                * of the string, and convert those.
!                */
<snip>
!               first = palloc(len + 1);
!               strncpy(first, init, len);
!               first[len] = '\0';

The use of palloc/pfree in this routine seems excessive. Does len have
upper bound? If so a simple array will do it.

!               if (strlen(first) < len)

Here you check the length of the remaining string and complain if it's
too short, but:

<snip>
!               result = strtol(first, &last, 10);
!               *src += (last - first);

Here you do not note if we didn't convert the entire string. So it
seems you are allowed to provide too few characters, as long as it's
not the last field?

Other than that it looks like a good patch.

Have a ncie day,
-- 
Martijn van Oosterhout   <kleptog(at)svana(dot)org>   http://svana.org/kleptog/
> Please line up in a tree and maintain the heap invariant while 
> boarding. Thank you for flying nlogn airlines.

In response to

Responses

pgsql-hackers by date

Next:From: Simon RiggsDate: 2008-09-09 09:35:35
Subject: Re: Synchronous Log Shipping Replication
Previous:From: Greg SmithDate: 2008-09-09 09:28:24
Subject: Re: [gsmith@gregsmith.com: Re: [patch] GUC source file and line number]

pgsql-patches by date

Next:From: Brendan JurdDate: 2008-09-09 11:04:39
Subject: Re: [PATCHES] to_date() validation
Previous:From: Ryan BradetichDate: 2008-09-09 06:25:31
Subject: Re: [PgFoundry] Unsigned Data Types [1 of 2]

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group