Hi -

> Thanks for the patch, some suggestions below.

Thanks, adding the following 'git diff -w' to the patch you reviewed:

> [...]
> I was experimenting with metadata queries to a local server that indexed
> a single rpm (binutils-2.41-8.fc40.x86_64.rpm). The following commands
> produced JSON with empty "results":
> 
>     debuginfod-find metadata glob '*'
>     debuginfod-find metadata glob '/usr*'
>     debuginfod-find metadata glob '/usr/bin*'
> 
> Using the glob '/usr/bin/*' did produce results with complete metadata.
> 
> I haven't looked into the cause but this seems like a bug.  I'd expect
> the first 3 globs to return at least as many results as the 4th. If this is
> intentional behaviour then I think it should be documented.

This is intentional.  When you use glob patterns like '*' or '/usr*'
in the shell, you don't get recursive path name lists either.
Addressed in the documentation via reference to shell /
fnmatch(FNM_PATHNAME).


diff --git a/debuginfod/debuginfod-client.c b/debuginfod/debuginfod-client.c
index c75abadf7dce..3d6f8d8c4bea 100644
--- a/debuginfod/debuginfod-client.c
+++ b/debuginfod/debuginfod-client.c
@@ -72,7 +72,7 @@ int debuginfod_find_section (debuginfod_client *c, const 
unsigned char *b,
                             int s, const char *scn, char **p)
                              { return -ENOSYS; }
 int debuginfod_find_metadata (debuginfod_client *c,
-                              const char *k, char *v, char **p) { return 
-ENOSYS; }
+                              const char *k, const char *v, char **p) { return 
-ENOSYS; }
 void debuginfod_set_progressfn(debuginfod_client *c,
                               debuginfod_progressfn_t fn) { }
 void debuginfod_set_verbose_fd(debuginfod_client *c, int fd) { }
@@ -858,22 +858,12 @@ init_server_urls(char* url_subdir, const char* type,
   char *server_url = strtok_r(server_urls, url_delim, &strtok_saveptr);
   /* Count number of URLs.  */
   int n = 0;
-  assert (0 == strcmp(url_subdir, "buildid") || 0 == strcmp(url_subdir, 
"metadata"));
 
   while (server_url != NULL)
     {
-      int r;
-      char *tmp_url;
-      if (strlen(server_url) > 1 && server_url[strlen(server_url)-1] == '/')
-        r = asprintf(&tmp_url, "%s%s", server_url, url_subdir);
-      else
-        r = asprintf(&tmp_url, "%s/%s", server_url, url_subdir);
-
-      if (r == -1)
-        return -ENOMEM;
-
-      // When we encounted a (well-formed) token off the form ima:foo, we 
update the policy
-      // under which results from that server will be ima verified
+      // When we encountered a (well-formed) token off the form
+      // ima:foo, we update the policy under which results from that
+      // server will be ima verified
       if (startswith(server_url, "ima:"))
         {
 #ifdef ENABLE_IMA_VERIFICATION
@@ -898,6 +888,17 @@ init_server_urls(char* url_subdir, const char* type,
           goto continue_next_url;
         }
 
+      // Construct actual URL for libcurl
+      int r;
+      char *tmp_url;
+      if (strlen(server_url) > 1 && server_url[strlen(server_url)-1] == '/')
+        r = asprintf(&tmp_url, "%s%s", server_url, url_subdir);
+      else
+        r = asprintf(&tmp_url, "%s/%s", server_url, url_subdir);
+
+      if (r == -1)
+        return -ENOMEM;
+      
       /* PR 27983: If the url is duplicate, skip it */
       int url_index;
       for (url_index = 0; url_index < n; ++url_index)
@@ -1025,7 +1026,7 @@ init_handle(debuginfod_client *client,
 
 /*
  * This function busy-waits on one or more curl queries to complete. This can
- * be controled via only_one, which, if true, will find the first winner and 
exit
+ * be controlled via only_one, which, if true, will find the first winner and 
exit
  * once found. If positive maxtime and maxsize dictate the maximum allowed 
wait times
  * and download sizes respectively. Returns 0 on success and -Posix error on 
failure.
  */
@@ -1044,7 +1045,7 @@ perform_queries(CURLM *curlm, CURL **target_handle, 
struct handle_data *data, de
       c->winning_headers = NULL;
     }
   if (maxtime > 0 && clock_gettime(CLOCK_MONOTONIC_RAW, &start_time) == -1)
-    return errno;
+    return -errno;
   long delta = 0;
   do
     {
@@ -1052,7 +1053,7 @@ perform_queries(CURLM *curlm, CURL **target_handle, 
struct handle_data *data, de
       if (maxtime > 0)
         {
           if (clock_gettime(CLOCK_MONOTONIC_RAW, &cur_time) == -1)
-            return errno;
+            return -errno;
           delta = cur_time.tv_sec - start_time.tv_sec;
           if ( delta >  maxtime)
             {
@@ -1108,9 +1109,7 @@ perform_queries(CURLM *curlm, CURL **target_handle, 
struct handle_data *data, de
         }
       
       long dl_size = -1;
-      if (only_one && target_handle)
-        { // Only bother with progress functions if we're retrieving exactly 1 
file
-          if (*target_handle && (c->progressfn || maxsize > 0))
+      if (target_handle && *target_handle && (c->progressfn || maxsize > 0))
         {
           /* Get size of file being downloaded. NB: If going through
              deflate-compressing proxies, this number is likely to be
@@ -1148,7 +1147,7 @@ perform_queries(CURLM *curlm, CURL **target_handle, 
struct handle_data *data, de
         {
           loops ++;
           long pa = loops; /* default param for progress callback */
-              if (*target_handle) /* we've committed to a server; report its 
download progress */
+          if (target_handle && *target_handle) /* we've committed to a server; 
report its download progress */
             {
               /* PR30809: Check actual size of cached file.  This same
                  fd is shared by all the multi-curl handles (but only
@@ -1186,7 +1185,6 @@ perform_queries(CURLM *curlm, CURL **target_handle, 
struct handle_data *data, de
                 break;
             }
         }
-        }
       /* Check to see if we are downloading something which exceeds maxsize, 
if set.*/
       if (target_handle && *target_handle && dl_size > maxsize && maxsize > 0)
         {
@@ -2050,7 +2048,6 @@ debuginfod_query_server_by_buildid (debuginfod_client *c,
     retry_limit = atoi (retry_limit_envvar);
 
   CURLM *curlm = c->server_mhandle;
-  assert (curlm != NULL);
 
   /* Tracks which handle should write to fd. Set to the first
      handle that is ready to write the target file to the cache.  */
@@ -2637,13 +2634,8 @@ debuginfod_find_section (debuginfod_client *client,
 
 
 int debuginfod_find_metadata (debuginfod_client *client,
-                              const char* key, char* value, char **path)
+                              const char* key, const char* value, char **path)
 {
-  (void) client;
-  (void) key;
-  (void) value;
-  (void) path;
-  
   char *server_urls = NULL;
   char *urls_envvar = NULL;
   char *cache_path = NULL;
@@ -2718,7 +2710,10 @@ int debuginfod_find_metadata (debuginfod_client *client,
   /* Check if we have a recent result already in the cache. */
   cache_path = make_cache_path();
   if (! cache_path)
+    {
+      rc = -ENOMEM;
       goto out;
+    }
   xalloc_str (target_cache_dir, "%s/metadata", cache_path);
   (void) mkdir (target_cache_dir, 0700);
   xalloc_str (target_cache_path, "%s/%s", target_cache_dir, target_file_name);
@@ -2823,7 +2818,6 @@ int debuginfod_find_metadata (debuginfod_client *client,
     }
 
   CURLM *curlm = client->server_mhandle;
-  assert (curlm != NULL);
 
   CURL *target_handle = NULL;
   data = malloc(sizeof(struct handle_data) * num_urls);
diff --git a/debuginfod/debuginfod-find.c b/debuginfod/debuginfod-find.c
index b0a7c2360dd8..0ef80377a81b 100644
--- a/debuginfod/debuginfod-find.c
+++ b/debuginfod/debuginfod-find.c
@@ -243,6 +243,8 @@ main(int argc, char** argv)
       
       rc = debuginfod_find_metadata (client, argv[remaining+1], 
argv[remaining+2],
                                      &cache_name);
+      if (rc >= 0)
+        {
           /* We output a pprinted JSON object, not the regular debuginfod-find 
cached file path */
           print_cached_file = false;
           json_object *metadata = json_object_from_file(cache_name);
@@ -262,6 +264,7 @@ main(int argc, char** argv)
               return 1;
             }
         }
+    }
   else
     {
       argp_help (&argp, stderr, ARGP_HELP_USAGE, argv[0]);
diff --git a/debuginfod/debuginfod.h.in b/debuginfod/debuginfod.h.in
index 3936b17b97cf..0a6a4a22efd9 100644
--- a/debuginfod/debuginfod.h.in
+++ b/debuginfod/debuginfod.h.in
@@ -97,17 +97,12 @@ int debuginfod_find_section (debuginfod_client *client,
    successful, set *path to a strdup'd copy of the name of the same
    file in the cache.  Caller must free() it later.
    
-   key can be one of 'glob' or 'file' corresponding to querying for value
-   by exact name or using a pattern matching approach.
-
-   The JSON document will be of the form {results: [{...}, ...], complete: 
<bool>},
-   where the results are JSON objects containing metadata and complete is true 
iff
-   all of the federation of servers responded with complete results (as 
opposed to 1+
-   failing to return or having an issue)
+   See the debuginfod-find(1) man page for examples of the supported types
+   of key/value queries and their JSON results.
    */
 int debuginfod_find_metadata (debuginfod_client *client,
                               const char *key,
-                              char* value,
+                              const char* value,
                               char **path);
 
 typedef int (*debuginfod_progressfn_t)(debuginfod_client *c, long a, long b);
diff --git a/doc/Makefile.am b/doc/Makefile.am
index 87de4f0beb7f..0c094af2289b 100644
--- a/doc/Makefile.am
+++ b/doc/Makefile.am
@@ -39,6 +39,7 @@ notrans_dist_man3_MANS += debuginfod_find_debuginfo.3
 notrans_dist_man3_MANS += debuginfod_find_executable.3
 notrans_dist_man3_MANS += debuginfod_find_source.3
 notrans_dist_man3_MANS += debuginfod_find_section.3
+notrans_dist_man3_MANS += debuginfod_find_metadata.3
 notrans_dist_man3_MANS += debuginfod_get_user_data.3
 notrans_dist_man3_MANS += debuginfod_get_url.3
 notrans_dist_man3_MANS += debuginfod_set_progressfn.3
diff --git a/doc/debuginfod-find.1 b/doc/debuginfod-find.1
index 8c63b2c5a5e0..38b5b0184dfe 100644
--- a/doc/debuginfod-find.1
+++ b/doc/debuginfod-find.1
@@ -123,16 +123,17 @@ l l.
 
 .SS metadata \fIKEY\fP \fIVALUE\fP
 
-All designated debuginfod servers are queried for metadata about files
-in their index.  Different search keys may be supported by different
-servers.
+All designated debuginfod servers are queried for metadata about all
+files that match a given key/value query in their index.  The results
+include names and buildids, which may be used in future queries to
+fetch actual files.
 
 .TS
 l l l .
 KEY    VALUE   DESCRIPTION
 
-\fBfile\fP     path    exact match \fIpath\fP, including in archives
-\fBglob\fP     pattern glob match \fIpattern\fP, including in archives
+\fBfile\fP     \fIpath\fP      exact match \fIpath\fP, including in archives
+\fBglob\fP     \fIpattern\fP   shell-style glob match \fIpattern\fP, including 
in archives, as in fnmatch(FNM_PATHNAME)
 .TE
 
 The resulting output will look something like the following
diff --git a/doc/debuginfod_find_debuginfo.3 b/doc/debuginfod_find_debuginfo.3
index 4e359c8c4bd4..589a2c2b63b4 100644
--- a/doc/debuginfod_find_debuginfo.3
+++ b/doc/debuginfod_find_debuginfo.3
@@ -48,6 +48,10 @@ LOOKUP FUNCTIONS
 .BI "                           int " build_id_len ","
 .BI "                           const char * " section ","
 .BI "                           char ** " path ");"
+.BI "int debuginfod_find_metadata(debuginfod_client *" client ","
+.BI "                            const char *" key ","
+.BI "                            const char *" value ","
+.BI "                            char ** " path ");"
 
 
 OPTIONAL FUNCTIONS
@@ -114,6 +118,14 @@ section queries, debuginfod_find_section may query the 
server for the
 debuginfo and/or executable with \fIbuild_id\fP in order to retrieve
 and extract the section.
 
+.BR debuginfod_find_metadata ()
+queries all debuginfod server URLs contained in
+.BR $DEBUGINFOD_URLS
+for metadata for all matches of a given key/value query against files
+in their indexes.  The resulting file is a JSON document.  See the
+\fIdebuginfod-find(1)\fP man page for examples of the supported types
+of key/value queries and their JSON results.
+
 If \fIpath\fP is not NULL and the query is successful, \fIpath\fP is set
 to the path of the file in the cache. The caller must \fBfree\fP() this value.
 
diff --git a/doc/debuginfod_find_metadata.3 b/doc/debuginfod_find_metadata.3
new file mode 100644
index 000000000000..16279936e2ea
--- /dev/null
+++ b/doc/debuginfod_find_metadata.3
@@ -0,0 +1 @@
+.so man3/debuginfod_find_debuginfo.3
diff --git a/tests/run-debuginfod-find-metadata.sh 
b/tests/run-debuginfod-find-metadata.sh
index f19c5a6e6942..78a34f09490f 100755
--- a/tests/run-debuginfod-find-metadata.sh
+++ b/tests/run-debuginfod-find-metadata.sh
@@ -1,6 +1,6 @@
 #!/usr/bin/env bash
 #
-# Copyright (C) 2022 Red Hat, Inc.
+# Copyright (C) 2022-2024 Red Hat, Inc.
 # This file is part of elfutils.
 #
 # This file is free software; you can redistribute it and/or modify




- FChE

Reply via email to