This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH 1/1] Add debuginfod support to GDB


>>>>> "Aaron" == Aaron Merey <amerey@redhat.com> writes:

Aaron> ChangeLog:
Aaron> 2020-01-08  Aaron Merey  <amerey@redhat.com>

Aaron>         * config/debuginfod.m4: New file. Add macro AC_DEBUGINFOD.
Aaron>         Adds new configure option --with-debuginfod.
Aaron>         * configure: Regenerate.
Aaron>         * configure.ac: Call AC_DEBUGINFOD.

Does the top-level configure really need AC_DEBUGINFOD?

If so, then this part of the patch has to go to gcc-patches first.
But, I suspect it's not needed, in which case dropping it is more
convenient, because we can have debuginfod.m4 locally and not involve
gcc at all.

Aaron> +if test "${with_debuginfod}" = no; then
Aaron> +  AC_MSG_WARN([debuginfod support disabled; some features may be unavailable.])
Aaron> +else
Aaron> +  AC_CHECK_LIB([debuginfod], [debuginfod_begin], [have_debuginfod_lib=yes])

Does and/or should libdebuginfod use pkg-config for this kind of thing?

Aaron> diff --git a/gdb/Makefile.in b/gdb/Makefile.in

Aaron> @@ -612,7 +615,7 @@ CLIBS = $(SIM) $(READLINE) $(OPCODES) $(BFD) $(LIBCTF) $(ZLIB) \
Aaron>  	@LIBS@ @GUILE_LIBS@ @PYTHON_LIBS@ \
Aaron>  	$(LIBEXPAT) $(LIBLZMA) $(LIBBABELTRACE) $(LIBIPT) \
Aaron>  	$(LIBIBERTY) $(WIN32LIBS) $(LIBGNU) $(LIBICONV) $(LIBMPFR) \
Aaron> -	$(SRCHIGH_LIBS) $(LIBXXHASH) $(PTHREAD_LIBS)
Aaron> +	$(SRCHIGH_LIBS) $(LIBXXHASH) $(PTHREAD_LIBS) $(LIBDEBUGINFOD)

Should it be before PTHREAD_LIBS?
If not, this is fine.

Aaron> +          /* Allow debuginfod to abort the download if SIGINT is raised.  */
Aaron> +          debuginfod_set_progressfn(

gdb style puts a space before the paren, and usually the paren is kept
next to the arguments if a newline is needed.

Aaron> +            client,
Aaron> +            [] (debuginfod_client *c, long a, long b)
Aaron> +              { return 1 ? check_quit_flag () : 0; }

This looks weird, because there's no need for the "1 ?" part.  Maybe it
should be just:

    return check_quit_flag ();

A lot of this code is duplicated in 3 places.  I think it would be
better to have a helper function to consolidate the shared code.

Aaron> +# Copyright 2010-2019 Free Software Foundation, Inc.

Probably just 2020.

Tom


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]