I've looked at this again, and you're right there isn't a leak. It wasn't clear to me what the ownership model was, but now I see that what was there before was fine. I personally prefer it with the OwningPtr, as it makes the memory management very clear, but it's up to you.
On Jan 19, 2010, at 9:02 AM, Daniel Dunbar wrote: > Hi Ted, > > On Mon, Jan 18, 2010 at 5:29 PM, Ted Kremenek <[email protected]> wrote: >> Author: kremenek >> Date: Mon Jan 18 19:29:05 2010 >> New Revision: 93834 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=93834&view=rev >> Log: >> Fix possible memory leak by using an OwningPtr. > > I can't see what the possible leak was, can you explain? > > - Daniel > >> Modified: >> cfe/trunk/lib/Driver/Driver.cpp >> >> Modified: cfe/trunk/lib/Driver/Driver.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Driver.cpp?rev=93834&r1=93833&r2=93834&view=diff >> >> ============================================================================== >> --- cfe/trunk/lib/Driver/Driver.cpp (original) >> +++ cfe/trunk/lib/Driver/Driver.cpp Mon Jan 18 19:29:05 2010 >> @@ -26,6 +26,7 @@ >> #include "clang/Basic/Version.h" >> >> #include "llvm/ADT/StringSet.h" >> +#include "llvm/ADT/OwningPtr.h" >> #include "llvm/Support/PrettyStackTrace.h" >> #include "llvm/Support/raw_ostream.h" >> #include "llvm/System/Path.h" >> @@ -675,7 +676,7 @@ >> } >> >> // Build the pipeline for this file. >> - Action *Current = new InputAction(*InputArg, InputType); >> + llvm::OwningPtr<Action> Current(new InputAction(*InputArg, InputType)); >> for (unsigned i = 0; i != NumSteps; ++i) { >> phases::ID Phase = types::getCompilationPhase(InputType, i); >> >> @@ -686,8 +687,7 @@ >> // Queue linker inputs. >> if (Phase == phases::Link) { >> assert(i + 1 == NumSteps && "linking must be final compilation >> step."); >> - LinkerInputs.push_back(Current); >> - Current = 0; >> + LinkerInputs.push_back(Current.take()); >> break; >> } >> >> @@ -698,14 +698,14 @@ >> continue; >> >> // Otherwise construct the appropriate action. >> - Current = ConstructPhaseAction(Args, Phase, Current); >> + Current.reset(ConstructPhaseAction(Args, Phase, Current.take())); >> if (Current->getType() == types::TY_Nothing) >> break; >> } >> >> // If we ended with something, add to the output list. >> if (Current) >> - Actions.push_back(Current); >> + Actions.push_back(Current.take()); >> } >> >> // Add a link action if necessary. >> >> >> _______________________________________________ >> cfe-commits mailing list >> [email protected] >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >> _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
