- User Since
- Jun 4 2013, 6:02 AM (302 w, 6 d)
Given the current way lldb command interpreter parser does things (which tries to model a posix shell), (one of) the correct ways to do '-quoting would be to replace each ' char by '\''.
I have a feeling most of these files should be just deleted, but since you went through the trouble of creating this patch, I see no reason why to not accept it.
LGTM, although this is looks like another piece of unmaintained code.
It looks like this code hasn't been touched since 2013, so who knows if it even works. But fixing this should be fine anyway.
Someone from the LLD(not B) project should review this.
It sounds to me like you could achieve the same thing by generalizing the LLDB_PYTHON_HOME logic in LLDBConfig.cmake. This would have the advantage of centralizing the way we manage python-finding logic (instead of each OS doing it's own thing) and also enable those users, who know what they are doing, to override this logic and point lldb to a different python. (I don't know if there are any such users, but it does not sounds like an impossible scenario).
Fri, Mar 22
This broke the DWO flavours of some tests:
lldb-Suite :: lang/c/const_variables/TestConstVariables.py lldb-Suite :: lang/c/local_variables/TestLocalVariables.py lldb-Suite :: lang/c/vla/TestVLA.py
Thu, Mar 21
minidump parser will go into llvm, which removes the main incentive for creating this new module. It's possible we may need something like this in the future, but should wait until there is a real use case for it.
instead of a fresh tool, minidump support will be added to obj2yaml.
minidump parser will go into llvm/Object instead.
This code will be in llvm now, so this cl is dead.
It occurred to me that the "invalid input" test cases could be easily rewritten
in lit, as they don't require parsing of the generated binary (because there
avoid heap allocations by using a stack buffer of fixed size which "should be enough for everyone".
I think this is fine, except for the things I pointed out inline. As for SafelyCloseFileDescriptor (wow) thing, I think we should use that, given that it's already there. I suggest making a separate patch for that.
Wed, Mar 20
The "problem" I have with this is that (IIRC) retrying close(2) on EINTR is the *wrong* thing to do on linux (because the fd will be closed anyway, and so we may end up closing someone else's file the second time around). I'll try to dig up more info about that tomorrow.
- rename Text/Hex streams
- fix Hex stream size handling and add some tests for it
- add sanitizer_common test
- fix the "write to freed memory" issue by redirecting the function call to a local buffer. To know the size of the buffer I need to allocate, I needed to add a new "platform limits" constant for the value of MB_LEN_MAX (another option would be to use some upper bound which is large enough for all reasonable platforms, such as 32).
Thanks for the review.
I think this should be fine, modulo the typo. TBH, I was not even aware some of these files existed. The ones that are already used by people on a daily basis should by already python3-ready.
Tue, Mar 19
The direction looks good to me too.
Mon, Mar 18
Updating to address review comments (thank you for the super quick turnaround).
Looks good to me. I have some comments inline and below, but none of them is really substantial.
I like the fact that we're moving the register methods into the respective class files. Among other things, this should make it easier for the instrumentation tool to insert register calls as well. However, I am not fond of introducing a public SB API call for something that should really be a private matter. One way to fix that would be to make the Initialize function private and make the SBReproducer class a friend, but perhaps it would be even better to not put this function into the public headers at all.