[PATCH] D112753: [llvm] [Support] Add CURL HTTP Client.

[Pavel Labath via Phabricator via llvm-commits]

labath added inline comments.


================
Comment at: llvm/include/llvm/Support/HTTPClient.h:83
+  void *Curl = nullptr;
+  Error curlInit();
+  ~HTTPClient();
----------------
noajshu wrote:
> Hi @labath, do you think we should follow the [[ https://llvm.org/docs/ProgrammersManual.html#fallible-constructors | style guide recommendation ]] for fallible constructors instead of using `curlInit`? That is, `HTTPClient` would have a private constructor and a `static Expected<HTTPClient> Create()`, which would just run the code inside `curlInit`.
Umm... maybe?

I don't think that doing this would be a bad choice, but for me, the more interesting question is whether we should rely on the implicit `curl_global_init` call that happens as a part of this. This seems to be discouraged by curl, and this kind of initialization is not a particularly nice thing for a library (which llvm is) to do.

If we can guarantee that `curl_global_init` was called, then we might not even need a fallible constructor here. The man page is not particularly clear on this, but I would expect that the only reason this can fail in this case is memory (or some other soft resource) exhaustion, and llvm is not robust against those anyway...


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

https://reviews.llvm.org/D112753