postgres.git / summary / log / commit / refs

commit    52c7a44e9518b3bbb930184526e0ae4eac412a7b
Author:   Tom Lane <tgl@sss.pgh.pa.us>
Date:     Sun Dec 01 19:15:37 2024 +0000

    Fix broken list-munging in ecpg's remove_variables().
    
    The loops over cursor argument variables neglected to ever advance
    "prevvar".  The code would accidentally do the right thing anyway
    when removing the first or second list entry, but if it had to
    remove the third or later entry then it would also remove all
    entries between there and the first entry.  AFAICS this would
    only matter for cursors that reference out-of-scope variables,
    which is a weird Informix compatibility hack; between that and
    the lack of impact for short lists, it's not so surprising that
    nobody has complained.  Nonetheless it's a pretty obvious bug.
    
    It would have been more obvious if these loops used a more standard
    coding style for chasing the linked lists --- this business with the
    "prev" pointer sometimes pointing at the current list entry is
    confusing and overcomplicated.  So rather than just add a minimal
    band-aid, I chose to rewrite the loops in the same style we use
    elsewhere, where the "prev" pointer is NULL until we are dealing with
    a non-first entry and we save the "next" pointer at the top of the
    loop.  (Two of the four loops touched here are not actually buggy,
    but it seems better to make them all look alike.)
    
    Coverity discovered this problem, but not until 2b41de4a5 added code
    to free no-longer-needed arguments structs.  With that, the incorrect
    link updates are possibly touching freed memory, and it complained
    about that.  Nonetheless the list corruption hazard is ancient, so
    back-patch to all supported branches.


 src/interfaces/ecpg/preproc/variable.c | 67 +++++++++++++++++-----------------
 1 file changed, 33 insertions(+), 34 deletions(-)

diff --git a/src/interfaces/ecpg/preproc/variable.c b/src/interfaces/ecpg/preproc/variable.c
index 8926676ab71..510c3ef1b06 100644
--- a/src/interfaces/ecpg/preproc/variable.c
+++ b/src/interfaces/ecpg/preproc/variable.c
@@ -260,33 +260,28 @@ void
 remove_typedefs(int brace_level)
 {
 	struct typedefs *p,
-			   *prev;
+			   *prev,
+			   *next;
 
-	for (p = prev = types; p;)
+	for (p = types, prev = NULL; p; p = next)
 	{
+		next = p->next;
 		if (p->brace_level >= brace_level)
 		{
 			/* remove it */
-			if (p == types)
-				prev = types = p->next;
+			if (prev)
+				prev->next = next;
 			else
-				prev->next = p->next;
+				types = next;
 
 			if (p->type->type_enum == ECPGt_struct || p->type->type_enum == ECPGt_union)
 				free(p->struct_member_list);
 			free(p->type);
 			free(p->name);
 			free(p);
-			if (prev == types)
-				p = types;
-			else
-				p = prev ? prev->next : NULL;
 		}
 		else
-		{
 			prev = p;
-			p = prev->next;
-		}
 	}
 }
 
@@ -294,63 +289,67 @@ void
 remove_variables(int brace_level)
 {
 	struct variable *p,
-			   *prev;
+			   *prev,
+			   *next;
 
-	for (p = prev = allvariables; p;)
+	for (p = allvariables, prev = NULL; p; p = next)
 	{
+		next = p->next;
 		if (p->brace_level >= brace_level)
 		{
-			/* is it still referenced by a cursor? */
+			/* remove it, but first remove any references from cursors */
 			struct cursor *ptr;
 
 			for (ptr = cur; ptr != NULL; ptr = ptr->next)
 			{
 				struct arguments *varptr,
-						   *prevvar;
+						   *prevvar,
+						   *nextvar;
 
-				for (varptr = prevvar = ptr->argsinsert; varptr != NULL; varptr = varptr->next)
+				for (varptr = ptr->argsinsert, prevvar = NULL;
+					 varptr != NULL; varptr = nextvar)
 				{
+					nextvar = varptr->next;
 					if (p == varptr->variable)
 					{
 						/* remove from list */
-						if (varptr == ptr->argsinsert)
-							ptr->argsinsert = varptr->next;
+						if (prevvar)
+							prevvar->next = nextvar;
 						else
-							prevvar->next = varptr->next;
+							ptr->argsinsert = nextvar;
 					}
+					else
+						prevvar = varptr;
 				}
-				for (varptr = prevvar = ptr->argsresult; varptr != NULL; varptr = varptr->next)
+				for (varptr = ptr->argsresult, prevvar = NULL;
+					 varptr != NULL; varptr = nextvar)
 				{
+					nextvar = varptr->next;
 					if (p == varptr->variable)
 					{
 						/* remove from list */
-						if (varptr == ptr->argsresult)
-							ptr->argsresult = varptr->next;
+						if (prevvar)
+							prevvar->next = nextvar;
 						else
-							prevvar->next = varptr->next;
+							ptr->argsresult = nextvar;
 					}
+					else
+						prevvar = varptr;
 				}
 			}
 
 			/* remove it */
-			if (p == allvariables)
-				prev = allvariables = p->next;
+			if (prev)
+				prev->next = next;
 			else
-				prev->next = p->next;
+				allvariables = next;
 
 			ECPGfree_type(p->type);
 			free(p->name);
 			free(p);
-			if (prev == allvariables)
-				p = allvariables;
-			else
-				p = prev ? prev->next : NULL;
 		}
 		else
-		{
 			prev = p;
-			p = prev->next;
-		}
 	}
 }
 


[parent: fa92c18efe24]