[Lldb-commits] [PATCH] D33035: Tool for using Intel(R) Processor Trace hardware feature

[Abhishek via Phabricator via lldb-commits]

abhishek.aggarwal added a comment.

In https://reviews.llvm.org/D33035#803801, @clayborg wrote:

> Much better on the API with Swig. Just a few inlined questions around GetRawBytes.


Hi Greg .. Thanks for feedback. My comments are inlined.



================
Comment at: tools/intel-features/intel-pt/PTDecoder.h:54-55
+
+  // Get raw bytes of the instruction
+  const uint8_t *GetRawBytes() const;
+
----------------
clayborg wrote:
> In the python version of this function, this should return a python string that contains the bytes or return None of there is nothing. Is that how it currently works? One alternate way of doing this is doing what we do with lldb::SBProcess:
> ```
>   size_t ReadMemory(addr_t addr, void *buf, size_t size, lldb::SBError &error);
> 
> ```
> 
> In python, we use a type map to detect the args "void *buf, size_t size", and we turn this into:
> 
> ```
> def ReadMemory(self, addr, error):
>     return # Python string with bytes from read memory
> ```
> 
> So one option would be to change GetRawBytes() into:
> 
> ```
> size_t GetRawBytes(void *buf, size_t size) const;
> ```
> 
> The return value is now many bytes were filled in. If "buf" is NULL or "size" is zero, you can return the length that you would need as the return value.
> 
> And have the python version be:
> 
> ```
> def GetRawBytes(self):
>     return # bytes in a python string or None
> ```
> 
Hi Greg.. As per my understanding, if we change ReadMemory() function as you proposed then both typemaps:
%typemap(in) (void *buf, size_t size) 
%typemap(argout) (void *buf, size_t size) 

will have to be defined. Am i right?

If it is so then I am afraid that I can't reuse these maps from lldb as the 2nd typemap refers to lldb_private::PythonBytes which is an internal library of lldb. Please let me know if I am wrong.


https://reviews.llvm.org/D33035