Hi,
On Fri, 2019-11-15 at 18:14 +, Pedro Alves wrote:
> On 11/15/19 5:35 PM, Mark Wielaard wrote:
>
> > IMHO it would be best to avoid any global state from the start. Since
> > we haven't released this api yet we can make it so that it is easy to
> > have state per request object.
>
> +1
>
> > In the gdb thread
> > https://sourceware.org/ml/gdb-patches/2019-11/msg00118.html Simon
> > Marchi suggested something like:
> >
> > debuginfod_client *client = debuginfod_create_client ();
> > debuginfod_set_progressfn (client, my_progress_cb);
> >
> > With debuginfod_client * being an opaque pointer. I think that is a
> > good idea. It also matches what libelf (with Elf *) and libdw (with
> > Dwarf *) do. It also makes it easy to add other kinds of state to
> > client requests later.
> >
>
> We were discussing this on the #gdb channel on IRC, and I think
> Simon's idea is the best one so far.
Attached is a variant that adds debuginfod_begin and debuginfo_end
(names matching elf/dwarf_begin/end) and adds a debuginfod_client
handle to each other function. The only state currently held is the
progressfn callback. But it could be extended to hold other state that
is now somewhat global (like the cache_path, server_urls, timeout which
come from the environment variables when set), or that are setup each
time a find call is made (like the handle_data array).
The full interface now looks as follows:
/* Names of environment variables that control the client logic. */
#define DEBUGINFOD_URLS_ENV_VAR "DEBUGINFOD_URLS"
#define DEBUGINFOD_CACHE_PATH_ENV_VAR "DEBUGINFOD_CACHE_PATH"
#define DEBUGINFOD_TIMEOUT_ENV_VAR "DEBUGINFOD_TIMEOUT"
/* Handle for debuginfod-client connection. */
typedef struct debuginfod_client debuginfod_client;
/* Create a handle for a new debuginfod-client session. */
debuginfod_client *debuginfod_begin (void);
/* Query the urls contained in $DEBUGINFOD_URLS for a file with
the specified type and build id. If build_id_len == 0, the
build_id is supplied as a lowercase hexadecimal string; otherwise
it is a binary blob of given legnth.
If successful, return a file descriptor to the target, otherwise
return a posix error code. If successful, set *path to a
strdup'd copy of the name of the same file in the cache.
Caller must free() it later. */
int debuginfod_find_debuginfo (debuginfod_client *client,
const unsigned char *build_id,
int build_id_len,
char **path);
int debuginfod_find_executable (debuginfod_client *client,
const unsigned char *build_id,
int build_id_len,
char **path);
int debuginfod_find_source (debuginfod_client *client,
const unsigned char *build_id,
int build_id_len,
const char *filename,
char **path);
typedef int (*debuginfod_progressfn_t)(debuginfod_client *c, long a, long b);
void debuginfod_set_progressfn(debuginfod_client *c,
debuginfod_progressfn_t fn);
/* Release debuginfod client connection context handle. */
void debuginfod_end (debuginfod_client *client);
I think the debuginfod_set_progressfn could use a bit more
documentation. It isn't immediately clear what a and b refer to. And
since we seem to use it in different phases maybe it needs an
"explanation message"?
For libdwfl I put all logic in a new file libdwfl/debuginfod-client.c
which sets up a debuginfod_client for each Dwfl that might need it.
The attached patch is against the debuginfod-submit branch on
sourceware elfutils.git.
Cheers,
Mark
From ed7a78599243a95d8723aef506ee865ce402f9f5 Mon Sep 17 00:00:00 2001
From: Mark Wielaard
Date: Sun, 17 Nov 2019 22:17:26 +0100
Subject: [PATCH] debuginfod client context
---
debuginfod/debuginfod-client.c | 72 --
debuginfod/debuginfod-find.c | 29 ++--
debuginfod/debuginfod.cxx| 35 ++---
debuginfod/debuginfod.h | 23 --
debuginfod/libdebuginfod.map | 2 +
libdwfl/Makefile.am | 2 +-
libdwfl/debuginfod-client.c | 122 +++
libdwfl/dwfl_build_id_find_elf.c | 37 +++---
libdwfl/dwfl_end.c | 2 +
libdwfl/find-debuginfo.c | 28 +--
libdwfl/libdwflP.h | 15
11 files changed, 267 insertions(+), 100 deletions(-)
create mode 100644 libdwfl/debuginfod-client.c
diff --git a/debuginfod/debuginfod-client.c b/debuginfod/debuginfod-client.c
index 9bcf1452..51f612ee 100644
--- a/debuginfod/debuginfod-client.c
+++ b/debuginfod/debuginfod-client.c
@@ -71,6 +71,16 @@
#include
#endif
+struct debuginfod_client
+{
+ /* Progress/interrupt callback function. */
+ debuginfod_progressfn_t progressfn;
+
+ /* Can contain all other context, like c
