On Mon, 2011-02-14 at 09:56 +0000, Paul Martin wrote: > On Mon, Feb 14, 2011 at 09:14:09AM +0100, Svante Signell wrote: .. > > + free(pattern); > > /* All checks have been passed; return true */ > > return 1; > > } > > You have a potential memory leak here. Only the "true" return does > free(pattern).
Thanks for your feedback, I was a little worried about memory leaks. Yes, the free statement should also be executed when the test fails. > > + newName = alloca(strlen(oldName)+1); > > strcpy(newName, oldName); > > > > + rotNames->disposeName = malloc(strlen(oldName)+1); > > strcpy(rotNames->disposeName, oldName); > > strdup(), perhaps? Yes, much better. I did not change the original strcpy statement. > > + newName = alloca(strlen(oldName)+1); > > newName = oldName; > > + oldName = alloca(strlen(tmp)+1); > > oldName = tmp; > > strdup()? Likewise! Then we should get rid of the alloca statements. > I'd be *very* hesitant in dynamically allocating from the stack at > runtime. Just because you have a generous stack allocation on i386 > doesn't mean that there is one on other architectures. If you overrun > the stack with alloca(), your program will hopefully segfault. > > Thanks for your contribution. I'll use this as the basis of a patch. BTW: I got a reply from upstream, saying that they would adopt my current patch. Maybe you can communicate with them directly about the new changes, in order to further improve the changes. -- To UNSUBSCRIBE, email to [email protected] with a subject of "unsubscribe". Trouble? Contact [email protected]
