patch 3/3 debuginfod client interruptability

[Mark Wielaard]

Hi,

On Fri, 2019-11-15 at 18:14 +0000, 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-debuginfod-client-context.patch
Type: text/x-patch
Size: 23594 bytes
Desc: not available
URL: <http://sourceware.org/pipermail/elfutils-devel/attachments/20191117/73d16b02/attachment.bin>