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

[Noah Shutty via Phabricator via llvm-commits]

noajshu added inline comments.


================
Comment at: llvm/lib/Support/HTTPClient.cpp:50-52
+#define HEADER_PREFIX "Content-Length: "
+#define HEADER_PREFIX_LEN 16
+  if (StringRef(Contents, HEADER_PREFIX_LEN) == HEADER_PREFIX) {
----------------
phosek wrote:
> I'd avoid the macros which we don't usually use in C++ and instead use the `StringRef` methods, so you can do something like:
> 
> ```
> StringRef Header(Contents, NMemb);
> if (Header.starts_with("Content-Length: ")) {
>   StringRef Length = Header.substr(16);
>   size_t ContentLength;
>   if (!Length.getAsInteger(10, ContentLength)) {
>     ...
>   }
> }
> ```
> 
> We should also make the code more resilient to handle potential issues, for example could the header look like `Content-Length    :     N` or is it guaranteed to always be `Content-Length: N`?
Thanks for pointing out the variations in header formatting. I took a look at the [[ https://www.ietf.org/rfc/rfc2616.txt | HTTP/1.1 specification ]] and changed the parser to use `startswith_insensitive` as header names are case insensitive as well.
To handle extra whitespace perhaps we could use a case-insensitive regex match like `content-length\s*?:\s*?(\d+)`. This would also give us the digits of the length as a match group.


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

https://reviews.llvm.org/D111252