amccarth added inline comments.
================
Comment at: source/Commands/CommandObjectTarget.cpp:2058
+ break;
+ }
num_dumped++;
----------------
Many LLVM developers prefer to omit the braces when the body of the
control-flow statement is a single statement.
================
Comment at: source/Interpreter/CommandInterpreter.cpp:2713
+ for (; chunk_size < size; ++chunk_size) {
+ assert(data[chunk_size] != '\0');
+ if (data[chunk_size] == '\n') {
----------------
Should we be that trusting of a caller? In a non-debug build, if a caller
doesn't end the (non-empty) string with '\n', this'll just run past the end of
the buffer. Did I miss something that guarantees the caller won't make a
mistake? Would it be too expensive to play it safe?
================
Comment at: source/Interpreter/CommandInterpreter.cpp:2720
+ chunk_size = stream.Write(data, chunk_size);
+ assert(size >= chunk_size);
+ data += chunk_size;
----------------
This assert should precede the line before it.
================
Comment at: tools/driver/Driver.cpp:1182
if (g_driver) {
- if (!g_interrupt_sent) {
- g_interrupt_sent = true;
+ if (!g_interrupt_sent.test_and_set()) {
g_driver->GetDebugger().DispatchInputInterrupt();
----------------
I'm not sure why these two ifs aren't one, as in:
```
if (g_driver && !g_interrupt_sent.test_and_set())
```
================
Comment at: tools/driver/Driver.cpp:1189
- exit(signo);
+ _exit(signo);
}
----------------
Can you add a comment explaining why this uses `_exit` rather than `exit`?
It's not obvious to me.
================
Comment at: tools/lldb-mi/MIDriverMain.cpp:71
//--
void sigint_handler(int vSigno) {
#ifdef _WIN32 // Restore handler as it is not persistent on Windows
----------------
I think this concurrency fix for SIGINT would be better in a separate patch. I
understand how it's related to the rest of this patch, but LLVM folks tend to
prefer small, incremental patches.
https://reviews.llvm.org/D37923
_______________________________________________
lldb-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
