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