[PATCH] debuginfod: Check all curl_easy_setopt calls

[PATCH] debuginfod: Check all curl_easy_setopt calls

[Mark Wielaard]

curl_easy_setup can fail for various reasons. Add a curl_easy_setopt_ck
macro to check all curl_easy_setopt calls and provides a human readable
error message in verbose mode.

Signed-off-by: Mark Wielaard <mark@klomp.org>
---
 debuginfod/ChangeLog           |  5 +++
 debuginfod/debuginfod-client.c | 61 ++++++++++++++++++++++------------
 2 files changed, 44 insertions(+), 22 deletions(-)

diff --git a/debuginfod/ChangeLog b/debuginfod/ChangeLog
index 91890786..983fd44a 100644
--- a/debuginfod/ChangeLog
+++ b/debuginfod/ChangeLog
@@ -1,3 +1,8 @@
+2022-05-09  Mark Wielaard  <mark@klomp.org>
+
+	* debuginfod-client.c (debuginfod_query_server): Add
+	curl_easy_setopt_ck macro, use it for all curl_easy_setopt calls.
+
 2022-05-09  Mark Wielaard  <mark@klomp.org>
 
 	* debuginfod-client.c (debuginfod_write_callback): Check result
diff --git a/debuginfod/debuginfod-client.c b/debuginfod/debuginfod-client.c
index 882a809a..89208216 100644
--- a/debuginfod/debuginfod-client.c
+++ b/debuginfod/debuginfod-client.c
@@ -1032,43 +1032,60 @@ debuginfod_query_server (debuginfod_client *c,
       if (vfd >= 0)
 	dprintf (vfd, "url %d %s\n", i, data[i].url);
 
+      /* Some boilerplate for checking curl_easy_setopt.  */
+#define curl_easy_setopt_ck(H,O,P) do {			\
+      CURLcode curl_res = curl_easy_setopt (H,O,P);	\
+      if (curl_res != CURLE_OK)				\
+	{						\
+	  if (vfd >= 0)					\
+	    dprintf (vfd,				\
+	             "Bad curl_easy_setopt: %s\n",	\
+		     curl_easy_strerror(curl_res));	\
+	  rc = -EINVAL;					\
+	  goto out2;					\
+	}						\
+      } while (0)
+
       /* Only allow http:// + https:// + file:// so we aren't being
 	 redirected to some unsupported protocol.  */
-      curl_easy_setopt(data[i].handle, CURLOPT_PROTOCOLS,
-		       CURLPROTO_HTTP | CURLPROTO_HTTPS | CURLPROTO_FILE);
-      curl_easy_setopt(data[i].handle, CURLOPT_URL, data[i].url);
+      curl_easy_setopt_ck(data[i].handle, CURLOPT_PROTOCOLS,
+			  (CURLPROTO_HTTP | CURLPROTO_HTTPS | CURLPROTO_FILE));
+      curl_easy_setopt_ck(data[i].handle, CURLOPT_URL, data[i].url);
       if (vfd >= 0)
-	curl_easy_setopt(data[i].handle, CURLOPT_ERRORBUFFER, data[i].errbuf);
-      curl_easy_setopt(data[i].handle,
-                       CURLOPT_WRITEFUNCTION,
-                       debuginfod_write_callback);
-      curl_easy_setopt(data[i].handle, CURLOPT_WRITEDATA, (void*)&data[i]);
+	curl_easy_setopt_ck(data[i].handle, CURLOPT_ERRORBUFFER,
+			    data[i].errbuf);
+      curl_easy_setopt_ck(data[i].handle,
+			  CURLOPT_WRITEFUNCTION,
+			  debuginfod_write_callback);
+      curl_easy_setopt_ck(data[i].handle, CURLOPT_WRITEDATA, (void*)&data[i]);
       if (timeout > 0)
 	{
 	  /* Make sure there is at least some progress,
 	     try to get at least 100K per timeout seconds.  */
-	  curl_easy_setopt (data[i].handle, CURLOPT_LOW_SPEED_TIME,
-			    timeout);
-	  curl_easy_setopt (data[i].handle, CURLOPT_LOW_SPEED_LIMIT,
-			    100 * 1024L);
+	  curl_easy_setopt_ck (data[i].handle, CURLOPT_LOW_SPEED_TIME,
+			       timeout);
+	  curl_easy_setopt_ck (data[i].handle, CURLOPT_LOW_SPEED_LIMIT,
+			       100 * 1024L);
 	}
       data[i].response_data = NULL;
       data[i].response_data_size = 0;
-      curl_easy_setopt(data[i].handle, CURLOPT_FILETIME, (long) 1);
-      curl_easy_setopt(data[i].handle, CURLOPT_FOLLOWLOCATION, (long) 1);
-      curl_easy_setopt(data[i].handle, CURLOPT_FAILONERROR, (long) 1);
-      curl_easy_setopt(data[i].handle, CURLOPT_NOSIGNAL, (long) 1);
-      curl_easy_setopt(data[i].handle, CURLOPT_HEADERFUNCTION, header_callback);
-      curl_easy_setopt(data[i].handle, CURLOPT_HEADERDATA, (void *) &(data[i]));
+      curl_easy_setopt_ck(data[i].handle, CURLOPT_FILETIME, (long) 1);
+      curl_easy_setopt_ck(data[i].handle, CURLOPT_FOLLOWLOCATION, (long) 1);
+      curl_easy_setopt_ck(data[i].handle, CURLOPT_FAILONERROR, (long) 1);
+      curl_easy_setopt_ck(data[i].handle, CURLOPT_NOSIGNAL, (long) 1);
+      curl_easy_setopt_ck(data[i].handle, CURLOPT_HEADERFUNCTION,
+			  header_callback);
+      curl_easy_setopt_ck(data[i].handle, CURLOPT_HEADERDATA,
+			  (void *) &(data[i]));
 #if LIBCURL_VERSION_NUM >= 0x072a00 /* 7.42.0 */
-      curl_easy_setopt(data[i].handle, CURLOPT_PATH_AS_IS, (long) 1);
+      curl_easy_setopt_ck(data[i].handle, CURLOPT_PATH_AS_IS, (long) 1);
 #else
       /* On old curl; no big deal, canonicalization here is almost the
          same, except perhaps for ? # type decorations at the tail. */
 #endif
-      curl_easy_setopt(data[i].handle, CURLOPT_AUTOREFERER, (long) 1);
-      curl_easy_setopt(data[i].handle, CURLOPT_ACCEPT_ENCODING, "");
-      curl_easy_setopt(data[i].handle, CURLOPT_HTTPHEADER, c->headers);
+      curl_easy_setopt_ck(data[i].handle, CURLOPT_AUTOREFERER, (long) 1);
+      curl_easy_setopt_ck(data[i].handle, CURLOPT_ACCEPT_ENCODING, "");
+      curl_easy_setopt_ck(data[i].handle, CURLOPT_HTTPHEADER, c->headers);
 
       curl_multi_add_handle(curlm, data[i].handle);
     }
-- 
2.18.4

Re: [PATCH] debuginfod: Check all curl_easy_setopt calls

[Mark Wielaard]

Hi,

On Mon, May 09, 2022 at 06:22:29PM +0200, Mark Wielaard wrote:
> curl_easy_setup can fail for various reasons. Add a curl_easy_setopt_ck
> macro to check all curl_easy_setopt calls and provides a human readable
> error message in verbose mode.

Pushed.

Cheers,

Mark