dbgclient: replace custom error codes with errno, modify user api

dbgserver/dbgserver-client.c: Custom error codes replaced with errno.
  dbgclient_build_id_find() replaced with
  dbgclient_find_{debuginfo,source,executable}().
  Removed dbgclient_enabled(), dbgclient_find_*() now returns
  -ENOENT if $DBGSERVER_URLS is not defined.
  dbgclient_query_server() now properly handles GET requests for
  source files.
dgbserver/dbgserver-client.h: Removed enum dbgclient_file_type and
  custom error codes. dbgclient_build_id_find() declaration replaced
  with dbgclient_find_{debuginfo,source,executable}().
libdwfl/dwfl_build_id_find_elf.c,find-debuginfo.c: remove
  dbgclient_enabled(), replace dbgclient_build_id_find with
  dbgclient_find_{executable,debuginfo}.

Signed-off-by: Aaron Merey <amerey@redhat.com>

4 files changed, 128 insertions, 118 deletions

diff --git a/dbgserver/dbgserver-client.c b/dbgserver/dbgserver-client.c
index 9602b591..921dab8d 100644
--- a/dbgserver/dbgserver-client.c
+++ b/dbgserver/dbgserver-client.c
@@ -3,6 +3,7 @@
 #include <dirent.h>
 #include <stdio.h>
 #include <stdlib.h>
+#include <errno.h>
 #include <unistd.h>
 #include <errno.h>
 #include <fcntl.h>
@@ -18,6 +19,7 @@
 #include <curl/curl.h>
 
 static const int max_build_id_bytes = 64;
+static int DBGCLIENT_OK = 0
 
 /* The cache_clean_interval_s file within the dbgclient cache specifies
    how frequently the cache should be cleaned. The file's st_mtime represents
@@ -36,12 +38,6 @@ static const char *server_urls_envvar = "DBGSERVER_URLS";
 static const char *url_delim =  " ";
 
 
-int
-dbgclient_enabled (void)
-{
-  return getenv(server_urls_envvar) != NULL;
-}
-
 static size_t
 dbgclient_write_callback (char *ptr, size_t size, size_t nmemb, void *fdptr)
 {
@@ -64,10 +60,10 @@ dbgclient_get_file_from_cache (char *target_cache_path)
   struct stat st;
 
   if (stat(target_cache_path, &st) == -1)
-    return DBGCLIENT_E_OK;
+    return -ENOENT;
 
   if ((fd = open(target_cache_path, O_RDONLY)) < 0)
-    return DBGCLIENT_E_CACHE_CANT_OPEN;
+    return -errno;
 
   return fd;
 }
@@ -83,22 +79,22 @@ dbgclient_init_cache (char *cache_path, char *interval_path)
 
   /* If the cache and config file already exist then we are done.  */
   if (stat(cache_path, &st) == 0 && stat(interval_path, &st) == 0)
-    return DBGCLIENT_E_OK;
+    return DBGCLIENT_OK;
 
   /* Create the cache and config file as necessary.  */
   if (stat(cache_path, &st) != 0 && mkdir(cache_path, 0777) < 0)
-    return DBGCLIENT_E_CACHE_CANT_OPEN;
+    return -errno;
 
   int fd;
   if (stat(interval_path, &st) != 0
       && (fd = open(interval_path, O_CREAT | O_RDWR, 0666)) < 0)
-    return DBGCLIENT_E_CACHE_CANT_CREATE;
+    return -errno;
 
   /* write default interval to config file.  */
   if (dprintf(fd, "%ld", cache_clean_default_interval_s) < 0)
-    return DBGCLIENT_E_CACHE_CANT_WRITE;
+    return -errno;
 
-  return DBGCLIENT_E_OK;
+  return DBGCLIENT_OK;
 }
 
 
@@ -113,11 +109,11 @@ dbgclient_add_file_to_cache (char *target_cache_dir,
 
   /* create target directory in cache if not found.  */
   if (stat(target_cache_dir, &st) == -1 && mkdir(target_cache_dir, 0777) < 0)
-    return DBGCLIENT_E_CACHE_CANT_CREATE;
+    return -errno;
 
   /* create target file if not found.  */
   if((fd = open(target_cache_path, O_CREAT | O_RDWR, 0666)) < 0)
-    return DBGCLIENT_E_CACHE_CANT_CREATE;
+    return -errno;
 
   return fd;
 }
@@ -137,13 +133,13 @@ dbgclient_clean_cache(char *cache_path, char *interval_path)
       interval_file = fopen(interval_path, "w");
 
       if (interval_file == NULL)
-        return DBGCLIENT_E_CACHE_CANT_CREATE;
+        return -errno;
 
       int rc = fprintf(interval_file, "%ld", cache_clean_default_interval_s);
       fclose(interval_file);
 
       if (rc < 0)
-        return DBGCLIENT_E_CACHE_CANT_WRITE;
+        return -errno;
     }
 
   /* Check timestamp of interval file to see whether cleaning is necessary.  */
@@ -155,13 +151,13 @@ dbgclient_clean_cache(char *cache_path, char *interval_path)
 
   if (time(NULL) - st.st_mtime < clean_interval)
     /* Interval has not passed, skip cleaning.  */
-    return DBGCLIENT_E_OK;
+    return DBGCLIENT_OK;
 
   char * const dirs[] = { cache_path, NULL, };
 
   FTS *fts = fts_open(dirs, 0, NULL);
   if (fts == NULL)
-    return DBGCLIENT_E_CACHE_CANT_OPEN;
+    return -errno;
 
   FTSENT *f;
   DIR *d;
@@ -193,12 +189,62 @@ dbgclient_clean_cache(char *cache_path, char *interval_path)
 
   /* Update timestamp representing when the cache was last cleaned.  */
   utime(interval_path, NULL);
-  return DBGCLIENT_E_OK;
+  return DBGCLIENT_OK;
+}
+
+
+/* Return value must be manually free'd.  */
+static char *
+build_url(char *server_url, char *build_id, char *type, char *filename)
+{
+  char *url;
+
+  if (filename != NULL)
+    {
+      url = malloc(strlen(server_url)
+                   + strlen("/buildid///")
+                   + strlen(build_id)
+                   + strlen(type)
+                   + strlen(filename)
+                   + 1);
+
+      if (url == NULL)
+        return NULL;
+
+      sprintf(url,
+              "%s/buildid/%s/%s/%s",
+              server_url,
+              build_id,
+              type,
+              filename);
+    }
+  else
+    {
+      url = malloc(strlen(server_url)
+                   + strlen("/buildid//")
+                   + strlen(build_id)
+                   + strlen(type)
+                   + 1);
+
+      if (url == NULL)
+        return NULL;
+
+      sprintf(url, "%s/buildid/%s/%s", server_url, build_id, type);
+    }
+
+  return url;
 }
 
 
+/* Query each of the server URLs found in $DBGSERVER_URLS for the file
+   with the specified build-id, type (debuginfo, executable or source)
+   and filename. filename may be NULL. If found, return a file
+   descriptor for the target, otherwise return an error code.  */
 static int
-dbgclient_query_server (char *build_id, char *type)
+dbgclient_query_server (const unsigned char *build_id_bytes,
+                        int build_id_len,
+                        char *type,
+                        char *filename)
 {
   char *urls_envvar;
   char *server_urls;
@@ -206,26 +252,31 @@ dbgclient_query_server (char *build_id, char *type)
   char interval_path[PATH_MAX];
   char target_cache_dir[PATH_MAX];
   char target_cache_path[PATH_MAX];
+  char build_id[max_build_id_bytes * 2 + 1];
+
+  /* Copy lowercase hex representation of build_id into buf.  */
+  for (int i = 0; i < build_id_len; i++)
+    sprintf(build_id + (i * 2), "%02x", build_id_bytes[i]);
 
   urls_envvar = getenv(server_urls_envvar);
   if (urls_envvar == NULL)
-    return DBGCLIENT_E_NOT_ENABLED;
+    return -ENOENT;
 
   /* make a copy of the envvar so it can be safely modified.  */
   server_urls = malloc(strlen(urls_envvar) + 1);
   if (server_urls == NULL)
-    return DBGCLIENT_E_OUT_OF_MEMORY;
+    return -ENOMEM;
 
   strcpy(server_urls, urls_envvar);
 
   if (curl_global_init(CURL_GLOBAL_DEFAULT) != 0)
-    return DBGCLIENT_E_CANT_INIT_CONNECTION;
+    return -ENETUNREACH;
 
   CURL *session = curl_easy_init();
   if (session == NULL)
     {
       curl_global_cleanup();
-      return DBGCLIENT_E_CANT_INIT_CONNECTION;
+      return -ENETUNREACH;
     }
 
   /* set paths needed to perform the query
@@ -260,11 +311,11 @@ dbgclient_query_server (char *build_id, char *type)
           PATH_MAX - strlen(interval_path));
 
   int rc = dbgclient_init_cache(cache_path, interval_path);
-  if (rc != DBGCLIENT_E_OK)
+  if (rc != DBGCLIENT_OK)
     return rc;
 
   rc = dbgclient_clean_cache(cache_path, interval_path);
-  if (rc != DBGCLIENT_E_OK)
+  if (rc != DBGCLIENT_OK)
     return rc;
 
   /* If the target is already in the cache then we are done.  */
@@ -285,20 +336,14 @@ dbgclient_query_server (char *build_id, char *type)
     {
       /* query servers until we find the target or run out of urls to try.  */
       long resp_code;
-      char *url = malloc(strlen(server_url)
-                  + strlen("/buildid//")
-                  + strlen(build_id)
-                  + strlen(type)
-                  + 1);
+      char *url = build_url(server_url, build_id, type, filename);
 
-      if (server_url == NULL)
+      if (url == NULL)
         {
           close(fd);
-          return DBGCLIENT_E_OUT_OF_MEMORY;
+          return -ENOMEM;
         }
 
-      sprintf(url, "%s/buildid/%s/%s", server_url, build_id, type);
-
       curl_easy_setopt(session, CURLOPT_URL, url);
       curl_easy_setopt(session,
                        CURLOPT_WRITEFUNCTION,
@@ -327,7 +372,6 @@ dbgclient_query_server (char *build_id, char *type)
   curl_easy_cleanup(session);
   curl_global_cleanup();
 
-
   if (! success)
     {
       close(fd);
@@ -340,39 +384,35 @@ dbgclient_query_server (char *build_id, char *type)
       if (readdir(d) == NULL)
         remove(target_cache_dir);
       closedir(d);
-      return DBGCLIENT_E_TARGET_NOT_FOUND;
+      return -ENOENT;
     }
 
   return fd;
 }
 
+/* See dbgserver-client.h  */
+int
+dbgclient_find_debuginfo (const unsigned char *build_id_bytes, int build_id_len)
+{
+  return dbgclient_query_server(build_id_bytes, build_id_len,
+                                "debuginfo", NULL);
+}
+
 
+/* See dbgserver-client.h  */
 int
-dbgclient_build_id_find (enum dbgclient_file_type file_type,
-                         const unsigned char *build_id,
-                         int build_id_len)
+dbgclient_find_executable(const unsigned char *build_id_bytes, int build_id_len)
 {
-  char *type;
-  char id_buf[max_build_id_bytes + 1];
+  return dbgclient_query_server(build_id_bytes, build_id_len,
+                                "executable", NULL);
+}
 
-  /* copy hex representation of buildid into id_buf.  */
-  for (int i = 0; i < build_id_len; i++)
-    sprintf(id_buf + (i * 2), "%02x", build_id[i]);
-
-  switch (file_type)
-  {
-  case dbgclient_file_type_debuginfo:
-    type = "debuginfo";
-    break;
-  case dbgclient_file_type_executable:
-    type = "executable";
-    break;
-  case dbgclient_file_type_source:
-    type = "source";
-    break;
-  default:
-    assert(0);
-  }
-
-  return dbgclient_query_server(id_buf, type);
+
+/* See dbgserver-client.h  */
+int dbgclient_find_source(const unsigned char *build_id_bytes,
+                          int build_id_len,
+                          char *filename)
+{
+  return dbgclient_query_server(build_id_bytes, build_id_len,
+                                "source-file", filename);
 }
diff --git a/dbgserver/dbgserver-client.h b/dbgserver/dbgserver-client.h
index 78faceec..d14435c2 100644
--- a/dbgserver/dbgserver-client.h
+++ b/dbgserver/dbgserver-client.h
@@ -1,34 +1,13 @@
-/* Return codes.  */
-#define DBGCLIENT_E_OK -1
-/* Client is not enabled for use.  */
-#define DBGCLIENT_E_NOT_ENABLED -2
-/* Cannot create either a file in the cache or the cache itself.  */
-#define DBGCLIENT_E_CACHE_CANT_CREATE -3
-/* Cannot open either a file in the cache or the cache itself.  */
-#define DBGCLIENT_E_CACHE_CANT_OPEN -4
-/* Cannot read either a file in the cache or the cache itself.  */
-#define DBGCLIENT_E_CACHE_CANT_READ -5
-/* Cannot write either a file in the cache or the cache itself.  */
-#define DBGCLIENT_E_CACHE_CANT_WRITE -6
-/* Out of memory.  */
-#define DBGCLIENT_E_OUT_OF_MEMORY -7
-/* Client was unable to locate the target on any dbgserver.  */
-#define DBGCLIENT_E_TARGET_NOT_FOUND -8
-/* Early init code failed, cannot connect with any dbgserver.  */
-#define DBGCLIENT_E_CANT_INIT_CONNECTION -9
-
-/* Indicates the type of target file.  */
-enum dbgclient_file_type {
-  dbgclient_file_type_debuginfo,
-  dbgclient_file_type_executable,
-  dbgclient_file_type_source,
-};
-
-/* Returns 1 if $DBGSERVER_URLS is defined, otherwise 0.  */
-int dbgclient_enabled (void);
-
 /* Query the urls contained in $DBGSERVER_URLS for a file with
-   the specified type and build id.  */
-int dbgclient_build_id_find (enum dbgclient_file_type type,
-                             const unsigned char *build_id,
+   the specified type and build id. If successful, return
+   a file descriptor to the target, otherwise return an
+   error code */
+int dbgclient_find_debuginfo (const unsigned char *build_id_bytes,
                              int build_id_len);
+
+int dbgclient_find_executable (const unsigned char *build_id_bytes,
+                               int build_id_len);
+
+int dbgclient_find_source (const unsigned char *build_id_bytes,
+                           int build_id_len,
+                           char *filename);
diff --git a/libdwfl/dwfl_build_id_find_elf.c b/libdwfl/dwfl_build_id_find_elf.c
index 2b6ecf75..b8e80127 100644
--- a/libdwfl/dwfl_build_id_find_elf.c
+++ b/libdwfl/dwfl_build_id_find_elf.c
@@ -192,27 +192,22 @@ dwfl_build_id_find_elf (Dwfl_Module *mod,
 #if ENABLE_DBGSERVER
   else {
     static void *dbgclient_so;
-    static __typeof__ (dbgclient_enabled) *fp_dbgclient_enabled;
-    static __typeof__ (dbgclient_build_id_find) *fp_dbgclient_build_id_find;
+    static __typeof__ (dbgclient_find_executable) *fp_dbgclient_find_executable;
 
     if (dbgclient_so == NULL)
       dbgclient_so = dlopen("libdbgserver-" VERSION ".so", RTLD_LAZY);
     if (dbgclient_so == NULL)
       dbgclient_so = dlopen("libdbgserver.so", RTLD_LAZY);
-    if (dbgclient_so != NULL && fp_dbgclient_enabled == NULL)
-      fp_dbgclient_enabled = dlsym (dbgclient_so, "dbgclient_enabled");
-    if (dbgclient_so != NULL && fp_dbgclient_build_id_find == NULL)
-      fp_dbgclient_build_id_find = dlsym (dbgclient_so, "dbgclient_build_id_find");
+    if (dbgclient_so != NULL && fp_dbgclient_find_executable == NULL)
+      fp_dbgclient_find_executable = dlsym (dbgclient_so, "dbgclient_find_executable");
 
-    if (fp_dbgclient_enabled != NULL && fp_dbgclient_build_id_find != NULL)
+    if (fp_dbgclient_find_executable != NULL)
       {
         /* If all else fails and a build-id is available, query the
            debuginfo-server if enabled.  */
-        if (fd < 0 && mod->build_id_len > 0 && (*fp_dbgclient_enabled)())
-          fd = (*fp_dbgclient_build_id_find) (dbgclient_file_type_executable,
-                                  mod->build_id_bits,
-                                  mod->build_id_len);
-
+        if (fd < 0 && mod->build_id_len > 0)
+          fd = (*fp_dbgclient_find_executable) (mod->build_id_bits,
+                                                mod->build_id_len);
       }
   }
 #endif /* ENABLE_DBGSERVER */
diff --git a/libdwfl/find-debuginfo.c b/libdwfl/find-debuginfo.c
index e084f7d1..5fbd511d 100644
--- a/libdwfl/find-debuginfo.c
+++ b/libdwfl/find-debuginfo.c
@@ -405,29 +405,25 @@ dwfl_standard_find_debuginfo (Dwfl_Module *mod,
 #if ENABLE_DBGSERVER
   {
     static void *dbgclient_so;
-    static __typeof__ (dbgclient_enabled) *fp_dbgclient_enabled;
-    static __typeof__ (dbgclient_build_id_find) *fp_dbgclient_build_id_find;
+    static __typeof__ (dbgclient_find_debuginfo) *fp_dbgclient_find_debuginfo;
 
     if (dbgclient_so == NULL)
       dbgclient_so = dlopen("libdbgserver-" VERSION ".so", RTLD_LAZY);
     if (dbgclient_so == NULL)
       dbgclient_so = dlopen("libdbgserver.so", RTLD_LAZY);
-    if (dbgclient_so != NULL && fp_dbgclient_enabled == NULL)
-      fp_dbgclient_enabled = dlsym (dbgclient_so, "dbgclient_enabled");
-    if (dbgclient_so != NULL && fp_dbgclient_build_id_find == NULL)
-      fp_dbgclient_build_id_find = dlsym (dbgclient_so, "dbgclient_build_id_find");
+    if (dbgclient_so != NULL && fp_dbgclient_find_debuginfo == NULL)
+      fp_dbgclient_find_debuginfo = dlsym (dbgclient_so, "dbgclient_find_debuginfo");
 
-    if (fp_dbgclient_enabled != NULL && fp_dbgclient_build_id_find != NULL)
+    if (fp_dbgclient_find_debuginfo != NULL)
       {
         /* If all else fails and a build-id is available, query the
            debuginfo-server if enabled.  */
-        if (fd < 0 && bits_len > 0 && (*fp_dbgclient_enabled)())
-          fd = (*fp_dbgclient_build_id_find) (dbgclient_file_type_debuginfo,
-                                              bits, bits_len);
+        if (fd < 0 && bits_len > 0)
+          fd = (*fp_dbgclient_find_debuginfo) (bits, bits_len);
       }
   }
 #endif /* ENABLE_DBGSERVER */
-    
+
   return fd;
 }
 INTDEF (dwfl_standard_find_debuginfo)