pset_quoted_string is broken

Started by David Rowleyover 11 years ago3 messageshackers
Jump to latest
#1David Rowley
dgrowleyml@gmail.com

It seems the buffer created in pset_quoted_string is just 1 char too small.

This breaks psql's \pset for me, though I've no idea why the buildfarm is
not complaining a bit more.

As it stands, if the function is given an empty string to quote, it tries
to build a string with 2 single quotes and a NUL. This needs 3 chars, not 2.

The attached simple patch fixes the problem.

Attachments:

pset_quoted_string_fix.difftext/plain; charset=US-ASCII; name=pset_quoted_string_fix.diffDownload+1-1
#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: David Rowley (#1)
Re: pset_quoted_string is broken

David Rowley <dgrowleyml@gmail.com> writes:

It seems the buffer created in pset_quoted_string is just 1 char too small.

Yeah, that's a bug. Fix pushed, thanks!

This breaks psql's \pset for me, though I've no idea why the buildfarm is
not complaining a bit more.

I think in most cases, maxalign padding of the malloc allocation would
result in there being room for another byte without trashing anything
important. You must be using a libc that notices and complains about
even 1-byte buffer overruns.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#3David Rowley
dgrowleyml@gmail.com
In reply to: Tom Lane (#2)
Re: pset_quoted_string is broken

On Mon, Oct 27, 2014 at 12:20 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

David Rowley <dgrowleyml@gmail.com> writes:

It seems the buffer created in pset_quoted_string is just 1 char too

small.

Yeah, that's a bug. Fix pushed, thanks!

Thanks for committing.

This breaks psql's \pset for me, though I've no idea why the buildfarm is
not complaining a bit more.

I think in most cases, maxalign padding of the malloc allocation would
result in there being room for another byte without trashing anything
important. You must be using a libc that notices and complains about
even 1-byte buffer overruns.

I'm using MSVC.
After a bit of reading it seems like when compiled in debug mode that
malloc() uses something called _malloc_dbg() which allocates a bit more
memory to allow for more strict checking of buffer overruns.

http://msdn.microsoft.com/en-us/library/974tc9t1.aspx

I guess all the MSVC buildfarm members must be compiled in release mode
then? I wonder if it would be worth changing one to build with debug as it
seem like none of the buildfarm animals picked this up despite there being
a regression test to ensure \pset works.

Regards

David Rowley