kwk added a comment. @jankratochvil thanks for this thorough review. I have to think about one comment more precisely but the rest was fixed.
================ Comment at: lldb/source/Host/common/DebugInfoD.cpp:50 + "invalid build ID: %s", + buildID.GetAsString("").c_str()); + ---------------- jankratochvil wrote: > It should not be an error: > ``` > echo 'int main(void) { return 0; }' >/tmp/main2.c;gcc -o /tmp/main2 > /tmp/main2.c -Wall -g -Wl,--build-id=none;rm > /tmp/main2.c;DEBUGINFOD_URLS=http://localhost:8002/ ./bin/lldb /tmp/main2 -o > 'l main' -o q > (lldb) target create "/tmp/main2" > Current executable set to '/tmp/main2' (x86_64). > (lldb) l main > warning: (x86_64) /tmp/main2 An error occurred while finding the source file > /tmp/main2.c using debuginfod for build ID A9C3D738: invalid build ID: > A9C3D738 > File: /tmp/main2.c > (lldb) q > ``` > Okay, I'll have it return just an empty string. And adjust the comment on the empty string in findSource documentation. I fully understand that an error is undesirable in your test case. My question is if the caller should sanitize it's parameters passed to `findSource` of if the latter should silently ignore those wrong UUIDs. For now I silently ignore them and treat a wrong build ID like a not found (e.g. empty string is returned). ================ Comment at: lldb/test/Shell/SymbolFile/DWARF/source-list.cpp:103 +// TEST-3: File: /my/new/path/test.cpp +// TEST-3: 123 +// TEST-3-NEXT: {{[0-9]+}} // Some context lines before ---------------- jankratochvil wrote: > `s/123/{{[0-9]+}}/?` Both are fine, but I'll go with your's if that helps. If you can tell me how to get a lit `CHECK` statement that checks for incremental numbers, that'll be awesome ;) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75750/new/ https://reviews.llvm.org/D75750 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits