[PATCH] D111252: [llvm] [Support] [Debuginfo] Add http and debuginfod client libraries and llvm-debuginfod-find tool

[Noah Shutty via Phabricator via llvm-commits]

noajshu marked 3 inline comments as done.
noajshu added inline comments.


================
Comment at: llvm/lib/Support/HTTPClient.cpp:51
+  llvm::Regex HeaderPattern(
+      "^content-length[[:space:]]*:[[:space:]]*([[:digit:]]+)[[:space:]]*$",
+      llvm::Regex::RegexFlags::IgnoreCase);
----------------
phosek wrote:
> According to the spec, `Content-Length` is the only acceptable spelling so I think we could be a little more strict here and require that particular case.
Agreed, it seems a safe bet and is much simpler than using a regex. I've switched it out to use nearly the exact same Content-Length parsing as clangd.


================
Comment at: llvm/lib/Support/HTTPClient.cpp:91
+
+  size_t ContentLengthParsed = std::stoll(ContentLength.str());
+  Buf->MemBuffer =
----------------
phosek wrote:
> Is it possible to use `StringRef::getAsInteger` instead?
Switched to use the clangd style parser which uses `llvm::getAsUnsignedInteger`.


================
Comment at: llvm/test/tools/llvm-debuginfod/llvm-debuginfod-find.test:2
+# REQUIRES: debuginfod_client
+# RUN: python %S/find-debuginfod-asset.py %S 13576 --executable
+# RUN: python %S/find-debuginfod-asset.py %S 13577 --debuginfo
----------------
phosek wrote:
> This would ideally be a substitution.
Thanks, I didn't realize this wasn't being expanded! I added the tool name in `llvm/test/lit.cfg.py` so that it gets substituted with the full path. If you'd like I can make this more explicit by using a % prefix and `ToolSubst`.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D111252/new/

https://reviews.llvm.org/D111252