NotificationsYou must be signed in to change notification settings
Fork4.9k
Star17.7k

Commit2f69645

committed

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 anywaywhen removing the first or second list entry, but if it had toremove the third or later entry then it would also remove allentries between there and the first entry. AFAICS this wouldonly matter for cursors that reference out-of-scope variables,which is a weird Informix compatibility hack; between that andthe lack of impact for short lists, it's not so surprising thatnobody has complained. Nonetheless it's a pretty obvious bug.It would have been more obvious if these loops used a more standardcoding style for chasing the linked lists --- this business with the"prev" pointer sometimes pointing at the current list entry isconfusing and overcomplicated. So rather than just add a minimalband-aid, I chose to rewrite the loops in the same style we useelsewhere, where the "prev" pointer is NULL until we are dealing witha non-first entry and we save the "next" pointer at the top of theloop. (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 until2b41de4 added codeto free no-longer-needed arguments structs. With that, the incorrectlink updates are possibly touching freed memory, and it complainedabout that. Nonetheless the list corruption hazard is ancient, soback-patch to all supported branches.

1 parente032e4c commit2f69645Copy full SHA for 2f69645

File tree

1 file changed

+29

-33

lines changed

src/interfaces/ecpg/preproc
- variable.c

1 file changed

+29

-33

lines changed

`‎src/interfaces/ecpg/preproc/variable.c`

Lines changed: 29 additions & 33 deletions

Original file line number	Diff line number	Diff line change
`@@ -264,17 +264,19 @@ void`
`264`	`264`	`remove_typedefs(intbrace_level)`
`265`	`265`	`{`
`266`	`266`	`structtypedefs*p,`
`267`		`-*prev;`
	`267`	`+*prev,`
	`268`	`+*next;`
`268`	`269`
`269`		`-for (p=prev=types;p;)`
	`270`	`+for (p=types,prev=NULL;p;p=next)`
`270`	`271`	`{`
	`272`	`+next=p->next;`
`271`	`273`	`if (p->brace_level >=brace_level)`
`272`	`274`	`{`
`273`	`275`	`/* remove it */`
`274`		`-if (p==types)`
`275`		`-prev=types=p->next;`
	`276`	`+if (prev)`
	`277`	`+prev->next=next;`
`276`	`278`	`else`
`277`		`-prev->next=p->next;`
	`279`	`+types=next;`
`278`	`280`
`279`	`281`	`if (p->type->type_enum==ECPGt_struct\|\|p->type->type_enum==ECPGt_union)`
`280`	`282`	`ECPGfree_struct_member(p->struct_member_list);`
`@@ -286,30 +288,25 @@ remove_typedefs(int brace_level)`
`286`	`288`	`free(p->type);`
`287`	`289`	`free(p->name);`
`288`	`290`	`free(p);`
`289`		`-if (prev==types)`
`290`		`-p=types;`
`291`		`-else`
`292`		`-p=prev ?prev->next :NULL;`
`293`	`291`	`}`
`294`	`292`	`else`
`295`		`-{`
`296`	`293`	`prev=p;`
`297`		`-p=prev->next;`
`298`		`-}`
`299`	`294`	`}`
`300`	`295`	`}`
`301`	`296`
`302`	`297`	`void`
`303`	`298`	`remove_variables(intbrace_level)`
`304`	`299`	`{`
`305`	`300`	`structvariable*p,`
`306`		`-*prev;`
	`301`	`+*prev,`
	`302`	`+*next;`
`307`	`303`
`308`		`-for (p=prev=allvariables;p;)`
	`304`	`+for (p=allvariables,prev=NULL;p;p=next)`
`309`	`305`	`{`
	`306`	`+next=p->next;`
`310`	`307`	`if (p->brace_level >=brace_level)`
`311`	`308`	`{`
`312`		`-/is it still referenced by a cursor? /`
	`309`	`+/remove it, but first remove any references from cursors /`
`313`	`310`	`structcursor*ptr;`
`314`	`311`
`315`	`312`	`for (ptr=cur;ptr!=NULL;ptr=ptr->next)`
`@@ -318,53 +315,52 @@ remove_variables(int brace_level)`
`318`	`315`	`*prevvar,`
`319`	`316`	`*nextvar;`
`320`	`317`
`321`		`-for (varptr=prevvar=ptr->argsinsert;varptr!=NULL;varptr=nextvar)`
	`318`	`+for (varptr=ptr->argsinsert,prevvar=NULL;`
	`319`	`+varptr!=NULL;varptr=nextvar)`
`322`	`320`	`{`
`323`	`321`	`nextvar=varptr->next;`
`324`	`322`	`if (p==varptr->variable)`
`325`	`323`	`{`
`326`	`324`	`/* remove from list */`
`327`		`-if (varptr==ptr->argsinsert)`
`328`		`-ptr->argsinsert=varptr->next;`
	`325`	`+if (prevvar)`
	`326`	`+prevvar->next=nextvar;`
`329`	`327`	`else`
`330`		`-prevvar->next=varptr->next;`
	`328`	`+ptr->argsinsert=nextvar;`
`331`	`329`	`free(varptr);`
`332`	`330`	`}`
	`331`	`+else`
	`332`	`+prevvar=varptr;`
`333`	`333`	`}`
`334`		`-for (varptr=prevvar=ptr->argsresult;varptr!=NULL;varptr=nextvar)`
	`334`	`+for (varptr=ptr->argsresult,prevvar=NULL;`
	`335`	`+varptr!=NULL;varptr=nextvar)`
`335`	`336`	`{`
`336`	`337`	`nextvar=varptr->next;`
`337`	`338`	`if (p==varptr->variable)`
`338`	`339`	`{`
`339`	`340`	`/* remove from list */`
`340`		`-if (varptr==ptr->argsresult)`
`341`		`-ptr->argsresult=varptr->next;`
	`341`	`+if (prevvar)`
	`342`	`+prevvar->next=nextvar;`
`342`	`343`	`else`
`343`		`-prevvar->next=varptr->next;`
	`344`	`+ptr->argsresult=nextvar;`
`344`	`345`	`free(varptr);`
`345`	`346`	`}`
	`347`	`+else`
	`348`	`+prevvar=varptr;`
`346`	`349`	`}`
`347`	`350`	`}`
`348`	`351`
`349`	`352`	`/* remove it */`
`350`		`-if (p==allvariables)`
`351`		`-prev=allvariables=p->next;`
	`353`	`+if (prev)`
	`354`	`+prev->next=next;`
`352`	`355`	`else`
`353`		`-prev->next=p->next;`
	`356`	`+allvariables=next;`
`354`	`357`
`355`	`358`	`ECPGfree_type(p->type);`
`356`	`359`	`free(p->name);`
`357`	`360`	`free(p);`
`358`		`-if (prev==allvariables)`
`359`		`-p=allvariables;`
`360`		`-else`
`361`		`-p=prev ?prev->next :NULL;`
`362`	`361`	`}`
`363`	`362`	`else`
`364`		`-{`
`365`	`363`	`prev=p;`
`366`		`-p=prev->next;`
`367`		`-}`
`368`	`364`	`}`
`369`	`365`	`}`
`370`	`366`

0 commit comments

Comments

(0)

Movatterモバイル変換

Navigation Menu

Search code, repositories, users, issues, pull requests...

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Commit2f69645

File tree

1 file changed

1 file changed

`‎src/interfaces/ecpg/preproc/variable.c`

0 commit comments