pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Alvaro Herrera <alvherre@2ndquadrant.com>
To: Daniel Verite <daniel@manitou-mail.org>
Cc: Tomas Vondra <tomas.vondra@2ndquadrant.com>
Cc: Craig Ringer <craig@2ndquadrant.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Robert Haas <robertmhaas@gmail.com>
Cc: Jim Nasby <Jim.Nasby@bluetreble.com>
Cc: Ronan Dunklau <ronan.dunklau@dalibo.com>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: Re: pg_dump / copy bugs with "big lines" ?
Date: Thu, 24 Nov 2016 17:39:12 -0300
Message-ID: <20161124203912.iccqstd4doqsnyuh@alvherre.pgsql> (raw)
In-Reply-To: <f98b2203-a5b6-4948-98c2-d425cf80f0b3@mm>
List-Unsubscribe: <mailto:majordomo@postgresql.org?body=unsub%20pgsql-hackers>

Daniel Verite wrote:

> Here's an updated patch. Compared to the previous version:
> 
> - removed CopyStartSend (per comment #1 in review)
> 
> - renamed flag to allow_long (comment #2)
> 
> - resetStringInfo no longer resets the flag (comment #3).
> 
> - allowLongStringInfo() is removed (comment #3 and #5),
> makeLongStringInfo() and initLongStringInfo() are provided
> instead, as alternatives to makeStringInfo() and initStringInfo().
> 
> - StringInfoData.len is back to int type, 2GB max.
> (addresses comment #4 incidentally).
> This is safer because  many routines peeking
> into StringInfoData use int for offsets into the buffer,
> for instance most of the stuff in backend/libpq/pqformat.c
> Altough these routines are not supposed to be called on long
> buffers, this assertion was not enforced in the code, and
> with a 64-bit length effectively over 2GB, they would misbehave
> pretty badly.

Hmm, this looks a lot better I think.  One change we overlooked and I
just noticed is that we need to change appendStringInfoVA() to return
size_t rather than int.  This comment at the bottom of it gave that up:

	/*
	 * Return pvsnprintf's estimate of the space needed.  (Although this is
	 * given as a size_t, we know it will fit in int because it's not more
	 * than MaxAllocSize.)
	 */
	return (int) nprinted;

The parenthical comment is no longer true.

This means we need to update all its callers to match.  I suppose it
won't be a problem for most of them since they are not using long
stringinfos anyway, but it seems better to keep everything consistent.
I hope that callers that do not adjust the type of the declared variable
would get a compiler warning.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


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



view thread (48+ messages)  latest in thread

Message-ID: <20161124203912.iccqstd4doqsnyuh@alvherre.pgsql>
Permalink:  ../20161124203912.iccqstd4doqsnyuh@alvherre.pgsql/
Also on:    postgresql.org/message-id/20161124203912.iccqstd4doqsnyuh@alvherre.pgsql

 · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-hackers@postgresql.org
  Cc: alvherre@2ndquadrant.com, daniel@manitou-mail.org, tomas.vondra@2ndquadrant.com, craig@2ndquadrant.com, tgl@sss.pgh.pa.us, robertmhaas@gmail.com, Jim.Nasby@bluetreble.com, ronan.dunklau@dalibo.com
  Subject: Re: pg_dump / copy bugs with "big lines" ?
  In-Reply-To: <20161124203912.iccqstd4doqsnyuh@alvherre.pgsql>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox