This is the mail archive of the
mailing list for the elfutils project.
Re: PR25369 rfc: debuginfod client api extension for progressfn prettying etc.
- From: Mark Wielaard <mark at klomp dot org>
- To: "Frank Ch. Eigler" <fche at redhat dot com>, elfutils-devel at sourceware dot org
- Cc: simark at simark dot ca
- Date: Tue, 25 Feb 2020 16:25:32 +0100
- Subject: Re: PR25369 rfc: debuginfod client api extension for progressfn prettying etc.
- References: <20200225033517.GA28949@redhat.com>
On Mon, 2020-02-24 at 22:35 -0500, Frank Ch. Eigler wrote:
> As a part of PR25369, I propose this small set of client api
> extensions, requested by gdb developers and needed by personal
> experience. (I plan to roll it out on my debuginfod servers shortly
> to let it soak.) An end-user visible immediate difference is in the
> DEBUGINFOD_PROGRESS=1 format message, which now looks like this:
> Downloading from debuginfod /
> Downloading from debuginfod -
> Downloading from debuginfod \
> Downloading from http://very:8222/buildid/817dcbd2ce0ecce19f22cbe5e136b6d9196428aa/executable 433130328/433130328
> This latter is a bit long and should probably be abbreviated, wdyt?
Might want to abbrev the build-id part to /81..aa/. The interesting
part is which server is used imho.
> I'd start reviewing this from the debuginfod.h header file outward.
OK. Lets start there:
> diff --git a/debuginfod/debuginfod.h b/debuginfod/debuginfod.h
> index b4b6a3d2a6b9..53aeee7133ca 100644
> --- a/debuginfod/debuginfod.h
> +++ b/debuginfod/debuginfod.h
> @@ -1,5 +1,5 @@
> /* External declarations for the libdebuginfod client library.
> - Copyright (C) 2019 Red Hat, Inc.
> + Copyright (C) 2019-2020 Red Hat, Inc.
> This file is part of elfutils.
> This file is free software; you can redistribute it and/or modify
> @@ -71,10 +71,23 @@ int debuginfod_find_source (debuginfod_client *client,
> const char *filename,
> char **path);
> +/* Set a progress callback function. */
> typedef int (*debuginfod_progressfn_t)(debuginfod_client *c, long a, long b);
> void debuginfod_set_progressfn(debuginfod_client *c,
> debuginfod_progressfn_t fn);
> +/* Add an outgoing HTTP request "Header: Value". Copies string. */
> +int debuginfod_add_http_header (debuginfod_client *client, const char* header);
This one seems different from the others and has a specific use case
just for the debuginfod server. Are you sure it is generic enough to be
added as a new public interface? If we add this can we do it separately
from other debuginfo-client progress improvements?
> +/* Return currently active URL, if known. String owned by curl, do not free. */
> +const char* debuginfod_get_url (debuginfod_client *client);
This does seem useful with the comment that was already made, that
lifetime of the returned string should be documented. I assume it is
valid to call this after debuginfod_find_* has returned, but before
debuginfod_end has been called?
> +/* Set the user parameter. */
> +void debuginfod_set_user_data (debuginfod_client *client, void *value);
> +/* Get the user parameter. */
> +void* debuginfod_get_user_data (debuginfod_client *client);
In theory I like these additions. But I don't really see the point of
how they are used. Is the only use case to pass the string "Progress"?
If there are no real users for this then I think we should not add
these at this time. Or is some other client using them? I am not really
against it, but would prefer if we add them separately to keep the