Re: [HACKERS] SQLDA fix for ECPG
On Sat, Nov 19, 2011 at 10:56:03PM +0100, Boszormenyi Zoltan wrote: Hopefully last turn in this topic. For NUMERIC types, the safe minimum alignment is a pointer because there are 5 int members followed by two pointer members in this struct. I got a crash from this with a lucky query and dataset. The DECIMAL struct is safe because the digits[] array is embedded there. ... Applied, thanks. Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Michael at BorussiaFan dot De, Meskes at (Debian|Postgresql) dot Org Jabber: michael.meskes at googlemail dot com VfL Borussia! Força Barça! Go SF 49ers! Use Debian GNU/Linux, PostgreSQL -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SQLDA fix for ECPG
Hi, 2011-11-17 14:53 keltezéssel, Michael Meskes írta: On Mon, Nov 14, 2011 at 09:06:30AM +0100, Boszormenyi Zoltan wrote: Yes, you are right. For timestamp and interval, the safe alignment is int64. Patch is attached. Applied, thanks. Michael thanks. Hopefully last turn in this topic. For NUMERIC types, the safe minimum alignment is a pointer because there are 5 int members followed by two pointer members in this struct. I got a crash from this with a lucky query and dataset. The DECIMAL struct is safe because the digits[] array is embedded there. After fixing this, I got another one at an innocent looking memcpy(): memcpy((char *) sqlda + offset, num-buf, num-ndigits + 1); It turned out that when the server sends 0.00, PGTYPESnumeric_from_asc() constructs a numeric structure with: ndigits == 0 buf == NULL digits == NULL. This makes memcpy() crash and burn. Let's also fix this case. Best regards, Zoltán Böszörményi -- -- Zoltán Böszörményi Cybertec Schönig Schönig GmbH Gröhrmühlgasse 26 A-2700 Wiener Neustadt, Austria Web: http://www.postgresql-support.de http://www.postgresql.at/ --- postgresql-9.1.1/src/interfaces/ecpg/ecpglib/sqlda.c.orig 2011-11-19 21:13:25.699169076 +0100 +++ postgresql-9.1.1/src/interfaces/ecpg/ecpglib/sqlda.c2011-11-19 22:50:13.895653223 +0100 @@ -110,7 +110,7 @@ * int Unfortunately we need to do double work here to compute * the size of the space needed for the numeric structure. */ - ecpg_sqlda_align_add_size(offset, sizeof(int), sizeof(numeric), offset, next_offset); + ecpg_sqlda_align_add_size(offset, sizeof(NumericDigit *), sizeof(numeric), offset, next_offset); if (!PQgetisnull(res, row, i)) { char *val = PQgetvalue(res, row, i); @@ -119,7 +119,8 @@ num = PGTYPESnumeric_from_asc(val, NULL); if (!num) break; - ecpg_sqlda_align_add_size(next_offset, sizeof(int), num-ndigits + 1, offset, next_offset); + if (num-ndigits) + ecpg_sqlda_align_add_size(next_offset, sizeof(int), num-ndigits + 1, offset, next_offset); PGTYPESnumeric_free(num); } break; @@ -323,7 +324,7 @@ set_data = false; - ecpg_sqlda_align_add_size(offset, sizeof(int), sizeof(numeric), offset, next_offset); + ecpg_sqlda_align_add_size(offset, sizeof(NumericDigit *), sizeof(numeric), offset, next_offset); sqlda-sqlvar[i].sqldata = (char *) sqlda + offset; sqlda-sqlvar[i].sqllen = sizeof(numeric); @@ -343,11 +344,14 @@ memcpy(sqlda-sqlvar[i].sqldata, num, sizeof(numeric)); - ecpg_sqlda_align_add_size(next_offset, sizeof(int), num-ndigits + 1, offset, next_offset); - memcpy((char *) sqlda + offset, num-buf, num-ndigits + 1); + if (num-ndigits) + { + ecpg_sqlda_align_add_size(next_offset, sizeof(int), num-ndigits + 1, offset, next_offset); + memcpy((char *) sqlda + offset, num-buf, num-ndigits + 1); - ((numeric *) sqlda-sqlvar[i].sqldata)-buf = (NumericDigit *) sqlda + offset; - ((numeric *) sqlda-sqlvar[i].sqldata)-digits = (NumericDigit *) sqlda + offset + (num-digits - num-buf); + ((numeric *) sqlda-sqlvar[i].sqldata)-buf = (NumericDigit *) sqlda + offset; + ((numeric *) sqlda-sqlvar[i].sqldata)-digits = (NumericDigit *) sqlda + offset + (num-digits - num-buf); + } PGTYPESnumeric_free(num); @@ -509,7 +513,7 @@ set_data = false; - ecpg_sqlda_align_add_size(offset, sizeof(int), sizeof(numeric), offset, next_offset); + ecpg_sqlda_align_add_size(offset, sizeof(NumericDigit *), sizeof(numeric), offset, next_offset);
Re: [HACKERS] SQLDA fix for ECPG
On Mon, Nov 14, 2011 at 09:06:30AM +0100, Boszormenyi Zoltan wrote: Yes, you are right. For timestamp and interval, the safe alignment is int64. Patch is attached. Applied, thanks. Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Michael at BorussiaFan dot De, Meskes at (Debian|Postgresql) dot Org Jabber: michael.meskes at googlemail dot com VfL Borussia! Força Barça! Go SF 49ers! Use Debian GNU/Linux, PostgreSQL -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SQLDA fix for ECPG
2011-11-13 17:27 keltezéssel, Tom Lane írta: Boszormenyi Zoltan z...@cybertec.at writes: I had a report about ECPG code crashing which involved a query using a date field. Attached is a one liner fix to make the date type's offset computed consistently across sqlda_common_total_size(), sqlda_compat_total_size() and sqlda_native_total_size(). Is this really the only issue there? I notice discrepancies among those three routines for some other types too, notably ECPGt_timestamp and ECPGt_interval. regards, tom lane Yes, you are right. For timestamp and interval, the safe alignment is int64. Patch is attached. Best regards, Zoltán Böszörményi -- -- Zoltán Böszörményi Cybertec Schönig Schönig GmbH Gröhrmühlgasse 26 A-2700 Wiener Neustadt, Austria Web: http://www.postgresql-support.de http://www.postgresql.at/ --- postgresql-9.1.1/src/interfaces/ecpg/ecpglib/sqlda.c.orig 2011-11-14 08:59:15.118711180 +0100 +++ postgresql-9.1.1/src/interfaces/ecpg/ecpglib/sqlda.c2011-11-14 09:02:53.787803059 +0100 @@ -127,10 +127,10 @@ sqlda_common_total_size(const PGresult * ecpg_sqlda_align_add_size(offset, sizeof(date), sizeof(date), offset, next_offset); break; case ECPGt_timestamp: - ecpg_sqlda_align_add_size(offset, sizeof(int), sizeof(timestamp), offset, next_offset); + ecpg_sqlda_align_add_size(offset, sizeof(int64), sizeof(timestamp), offset, next_offset); break; case ECPGt_interval: - ecpg_sqlda_align_add_size(offset, sizeof(int), sizeof(interval), offset, next_offset); + ecpg_sqlda_align_add_size(offset, sizeof(int64), sizeof(interval), offset, next_offset); break; case ECPGt_char: case ECPGt_unsigned_char: @@ -359,7 +359,7 @@ ecpg_set_compat_sqlda(int lineno, struct sqlda-sqlvar[i].sqllen = sizeof(date); break; case ECPGt_timestamp: - ecpg_sqlda_align_add_size(offset, sizeof(timestamp), sizeof(timestamp), offset, next_offset); + ecpg_sqlda_align_add_size(offset, sizeof(int64), sizeof(timestamp), offset, next_offset); sqlda-sqlvar[i].sqldata = (char *) sqlda + offset; sqlda-sqlvar[i].sqllen = sizeof(timestamp); break; @@ -545,7 +545,7 @@ ecpg_set_native_sqlda(int lineno, struct sqlda-sqlvar[i].sqllen = sizeof(date); break; case ECPGt_timestamp: - ecpg_sqlda_align_add_size(offset, sizeof(timestamp), sizeof(timestamp), offset, next_offset); + ecpg_sqlda_align_add_size(offset, sizeof(int64), sizeof(timestamp), offset, next_offset); sqlda-sqlvar[i].sqldata = (char *) sqlda + offset; sqlda-sqlvar[i].sqllen = sizeof(timestamp); break; -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SQLDA fix for ECPG
This must have been a cut and paste bug and is incorrect in 9.0.x, 9.1.x and GIT HEAD. It would be nice to have it applied before the next point releases come out. Applied, thanks for the patch. Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Michael at BorussiaFan dot De, Meskes at (Debian|Postgresql) dot Org Jabber: michael.meskes at googlemail dot com VfL Borussia! Força Barça! Go SF 49ers! Use Debian GNU/Linux, PostgreSQL -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SQLDA fix for ECPG
Boszormenyi Zoltan z...@cybertec.at writes: I had a report about ECPG code crashing which involved a query using a date field. Attached is a one liner fix to make the date type's offset computed consistently across sqlda_common_total_size(), sqlda_compat_total_size() and sqlda_native_total_size(). Is this really the only issue there? I notice discrepancies among those three routines for some other types too, notably ECPGt_timestamp and ECPGt_interval. 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