[PATCH] ECPG bug fix in preproc when indicator struct is shorter than record struct

Started by Rader, Davidabout 8 years ago4 messages
#1Rader, David
davidr@openscg.com
4 attachment(s)

Hello -

Attached is a proposed patch to fix a bug in the ECPG preprocessor that
generates application code that core dumps at run-time. When the input pgc
code uses a record struct for returning query results and uses an indicator
struct that has fewer fields than the record struct, the generated .c code
will compile with no warning but core dump. This situation comes up when a
developer adds a field to an existing query, adds the field to the record
struct and forgets to add the field to the indicator struct.

The patch fixes the generated code to use ECPGt_NO_INDICATOR in the call to
ecpglib for indicator members that are not present and issues a compiler
warning for either too few indicator members or too many indicator members.

The attached sample files are a simple sample of pgc code that can be used
to see the difference in before and after generation and the before and
after generated code.

If accepted, this bug fix can be back ported to earlier versions of ecpg as
well.

Thanks
Dave

Attachments:

indrecs.pgcapplication/octet-stream; name=indrecs.pgcDownload
0001-Fix-generated-code-to-avoid-core-dump-when-indicator.patchtext/x-patch; charset=US-ASCII; name=0001-Fix-generated-code-to-avoid-core-dump-when-indicator.patchDownload
From dc24cf9e4b4eb22be1e570fd449f97c6251d61c3 Mon Sep 17 00:00:00 2001
From: David Rader <davidr@openscg.com>
Date: Thu, 11 Jan 2018 16:26:57 +0000
Subject: [PATCH] Fix generated code to avoid core dump when indicator struct
 has fewer members than record struct. Add compiler warnings for too few or
 too many indicator struct members.

---
 src/interfaces/ecpg/preproc/type.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/src/interfaces/ecpg/preproc/type.c b/src/interfaces/ecpg/preproc/type.c
index 4abbf93..fa1a05c 100644
--- a/src/interfaces/ecpg/preproc/type.c
+++ b/src/interfaces/ecpg/preproc/type.c
@@ -609,7 +609,17 @@ ECPGdump_a_struct(FILE *o, const char *name, const char *ind_name, char *arrsize
 						prefix, ind_prefix, arrsize, type->struct_sizeof,
 						(ind_p != NULL) ? ind_type->struct_sizeof : NULL);
 		if (ind_p != NULL && ind_p != &struct_no_indicator)
+		{
 			ind_p = ind_p->next;
+			if (ind_p == NULL && p->next != NULL) {
+				mmerror(PARSE_ERROR, ET_WARNING, "indicator struct \"%s\" has too few members", ind_name);
+				ind_p = &struct_no_indicator;
+			}
+		}
+	}
+
+	if (ind_type != NULL && ind_p != NULL && ind_p != &struct_no_indicator) {
+		mmerror(PARSE_ERROR, ET_WARNING, "indicator struct \"%s\" has too many members", ind_name);
 	}
 
 	free(pbuf);
-- 
1.8.3.1

indrecs.ctext/x-csrc; charset=US-ASCII; name=indrecs.cDownload
indrecs.c.fixapplication/octet-stream; name=indrecs.c.fixDownload
#2Michael Meskes
meskes@postgresql.org
In reply to: Rader, David (#1)
Re: [PATCH] ECPG bug fix in preproc when indicator struct is shorter than record struct

Attached is a proposed patch to fix a bug in the ECPG preprocessor
that generates application code that core dumps at run-time. When the
input pgc code uses a record struct for returning query results and
uses an indicator struct that has fewer fields than the record
struct, the generated .c code will compile with no warning but core
dump. This situation comes up when a developer adds a field to an
existing query, adds the field to the record struct and forgets to
add the field to the indicator struct.

Thanks for spotting and fixing, committed.

The attached sample files are a simple sample of pgc code that can be
used to see the difference in before and after generation and the
before and after generated code.

Next time it would be nice if the test case was self-contained. Wasn't
that difficult to figure out the table layout, though. :)

If accepted, this bug fix can be back ported to earlier versions of
ecpg as well.

As usual this will be done after a couple of days, if no problems
appear. I'm pretty sure there won't but sticking to my workflow here.

Michael
--
Michael Meskes
Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org)
Meskes at (Debian|Postgresql) dot Org
Jabber: michael at xmpp dot meskes dot org
VfL Borussia! Força Barça! SF 49ers! Use Debian GNU/Linux, PostgreSQL

#3Rader, David
davidr@openscg.com
In reply to: Michael Meskes (#2)
Re: [PATCH] ECPG bug fix in preproc when indicator struct is shorter than record struct

Thank you!

On Sat, Jan 13, 2018 at 9:02 AM, Michael Meskes <meskes@postgresql.org>
wrote:

Attached is a proposed patch to fix a bug in the ECPG preprocessor
that generates application code that core dumps at run-time. When the
input pgc code uses a record struct for returning query results and
uses an indicator struct that has fewer fields than the record
struct, the generated .c code will compile with no warning but core
dump. This situation comes up when a developer adds a field to an
existing query, adds the field to the record struct and forgets to
add the field to the indicator struct.

Thanks for spotting and fixing, committed.

The attached sample files are a simple sample of pgc code that can be
used to see the difference in before and after generation and the
before and after generated code.

Next time it would be nice if the test case was self-contained. Wasn't
that difficult to figure out the table layout, though. :)

Got it - will add next time.

If accepted, this bug fix can be back ported to earlier versions of
ecpg as well.

As usual this will be done after a couple of days, if no problems
appear. I'm pretty sure there won't but sticking to my workflow here.

Do you want patches for the back ports as well?
I noticed that between 9.6 (which is what we're using with this customer)
and 10 the variable arrsiz was renamed to arrsize, so slight differences.
Did not check earlier releases yet.

Show quoted text

Michael
--
Michael Meskes
Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org)
Meskes at (Debian|Postgresql) dot Org
Jabber: michael at xmpp dot meskes dot org
VfL Borussia! Força Barça! SF 49ers! Use Debian GNU/Linux, PostgreSQL

#4Michael Meskes
meskes@postgresql.org
In reply to: Rader, David (#3)
Re: [PATCH] ECPG bug fix in preproc when indicator struct is shorter than record struct

Do you want patches for the back ports as well?
I noticed that between 9.6 (which is what we're using with this
customer) and 10 the variable arrsiz was renamed to arrsize, so
slight differences. Did not check earlier releases yet.

Na, don't worry, git cherry-pick and conflict resolution will do the
trick. But thanks for the heads-up.

Michael
--
Michael Meskes
Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org)
Meskes at (Debian|Postgresql) dot Org
Jabber: michael at xmpp dot meskes dot org
VfL Borussia! Força Barça! SF 49ers! Use Debian GNU/Linux, PostgreSQL