On Sun, Aug 10, 2025 at 8:27 PM Aaron Merey <ame...@redhat.com> wrote:
>
> Add a SIGINT handler to debuginfod-find so that ctrl-c during
> a download causes clean up to occur instead of immediately killing
> the process and possibly leaving temp files in the client cache.
>
> This change requires setting the client progressfn unconditionally
> instead of only when --verbose is given. This in turn requires a small
> change to a debuginfod-find testcase in run-debuginfod-artifact-running.sh
> to account for output differences between the debuginfod-find progressfn
> and the debuginfod_client default_progressfn which was previously used
> when debuginfod-find lacked -v but had DEBUGINFOD_PROGRESS=1 set.
>
> https://sourceware.org/bugzilla/show_bug.cgi?id=33163
>
> Signed-off-by: Aaron Merey <ame...@redhat.com>
Pushed as commit d7cb72f70377 after Frank reviewed on #elfutils IRC.
> ---
> debuginfod/debuginfod-find.c | 30 ++++++++++++++++++++++--
> tests/run-debuginfod-artifact-running.sh | 4 ++--
> 2 files changed, 30 insertions(+), 4 deletions(-)
>
> diff --git a/debuginfod/debuginfod-find.c b/debuginfod/debuginfod-find.c
> index deba9560..39de65c3 100644
> --- a/debuginfod/debuginfod-find.c
> +++ b/debuginfod/debuginfod-find.c
> @@ -30,6 +30,7 @@
> #include <fcntl.h>
> #include <gelf.h>
> #include <libdwelf.h>
> +#include <signal.h>
> #ifndef DUMMY_LIBDEBUGINFOD
> #include <json-c/json.h>
> #endif
> @@ -66,10 +67,22 @@ static const struct argp_option options[] =
> /* debuginfod connection handle. */
> static debuginfod_client *client;
> static int verbose;
> +static volatile sig_atomic_t interrupted;
> +
> +static void
> +handle_sigint(int signo __attribute__((__unused__)))
> +{
> + interrupted = 1;
> +}
>
> int progressfn(debuginfod_client *c __attribute__((__unused__)),
> long a, long b)
> {
> + if (interrupted)
> + return 1;
> + if (verbose < 1)
> + return 0;
> +
> static bool first = true;
> static struct timespec last;
> struct timespec now;
> @@ -103,7 +116,6 @@ static error_t parse_opt (int key, char *arg, struct
> argp_state *state)
> switch (key)
> {
> case 'v': verbose++;
> - debuginfod_set_progressfn (client, & progressfn);
> if (verbose > 1)
> debuginfod_set_verbose_fd (client, STDERR_FILENO);
> break;
> @@ -133,6 +145,16 @@ main(int argc, char** argv)
> return 1;
> }
>
> + /* Set SIGINT handler and progressfn so that temp files can be cleaned
> + up when a download is cancelled. */
> + struct sigaction sa;
> + sigemptyset (&sa.sa_mask);
> + sa.sa_flags = 0;
> + sa.sa_handler = handle_sigint;
> + sigaction (SIGINT, &sa, NULL);
> +
> + debuginfod_set_progressfn (client, & progressfn);
> +
> /* Exercise user data pointer, to support testing only. */
> debuginfod_set_user_data (client, (void *)"Progress");
>
> @@ -291,7 +313,11 @@ main(int argc, char** argv)
>
> if (rc < 0)
> {
> - fprintf(stderr, "Server query failed: %s\n", strerror(-rc));
> + if (interrupted != 0)
> + fputs ("Server query cancelled\n", stderr);
> + else
> + fprintf(stderr, "Server query failed: %s\n", strerror(-rc));
> +
> return 1;
> }
> else
> diff --git a/tests/run-debuginfod-artifact-running.sh
> b/tests/run-debuginfod-artifact-running.sh
> index df42957b..2d5d834d 100755
> --- a/tests/run-debuginfod-artifact-running.sh
> +++ b/tests/run-debuginfod-artifact-running.sh
> @@ -105,10 +105,10 @@ cmp $filename F/prog2
> grep -q Progress vlog
> grep -q Downloaded.from vlog
> tempfiles vlog
> -filename=`testrun env DEBUGINFOD_PROGRESS=1
> ${abs_top_builddir}/debuginfod/debuginfod-find executable $BUILDID2 2>vlog2`
> +filename=`testrun env DEBUGINFOD_PROGRESS=1
> ${abs_top_builddir}/debuginfod/debuginfod-find -v executable $BUILDID2
> 2>vlog2`
> cmp $filename F/prog2
> -grep -q 'Downloading.*http' vlog2
> tempfiles vlog2
> +grep -q 'Progress' vlog2
> filename=`testrun ${abs_top_builddir}/debuginfod/debuginfod-find source
> $BUILDID2 ${PWD}/prog2.c`
> cmp $filename ${PWD}/prog2.c
>
> --
> 2.50.1
>
