[Lldb-commits] [PATCH] D18459: Fix FILE * leak in Python API

[Greg Clayton via lldb-commits]

clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.

I would prefer to do this with overloading if possible. See inlined comments.


================
Comment at: include/lldb/API/SBCommandReturnObject.h:87-91
@@ -86,7 +86,7 @@
     
     void
     SetImmediateOutputFile (FILE *fh);
     
     void
     SetImmediateErrorFile (FILE *fh);
     
----------------
If we end up using overloading, see below, then these should be marked as deprecated with a comment. We should also add some header doc to specify that the above two functions do not take ownership of the file handle.

================
Comment at: include/lldb/API/SBCommandReturnObject.h:93-97
@@ -92,2 +92,7 @@
     
     void
+    SetImmediateOutputFileWithOwnership(FILE* file);
+    
+    void
+    SetImmediateErrorFileWithOwnership(FILE* file);
+    
----------------
It might be better to just overload:

```    
    void
    SetImmediateOutputFile(FILE *fh, bool transfer_ownership);

    void
    SetImmediateErrorFile (FILE *fh, bool transfer_ownership);
```



================
Comment at: scripts/interface/SBCommandReturnObject.i:87-94
@@ -86,7 +86,10 @@
     
+    %rename(SetImmediateOutputFile) SetImmediateOutputFileWithOwnership;
+    %rename(SetImmediateErrorFile) SetImmediateErrorFileWithOwnership;
+
     void
-    SetImmediateOutputFile (FILE *fh);
-    
+    SetImmediateOutputFileWithOwnership (FILE *fh);
+
     void
-    SetImmediateErrorFile (FILE *fh);
+    SetImmediateErrorFileWithOwnership (FILE *fh);
 
----------------
Not sure if we can do this if we overload.


http://reviews.llvm.org/D18459