[cfe-commits] r93834 - /cfe/trunk/lib/Driver/Driver.cpp

[Ted Kremenek]

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 <kremenek at apple.com> 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
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>