From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 111789 invoked by alias); 21 Aug 2019 17:32:17 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 111721 invoked by uid 89); 21 Aug 2019 17:32:16 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-15.6 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,SPF_HELO_PASS autolearn=ham version=3.3.1 spammy= X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 21 Aug 2019 17:32:06 +0000 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 2363E8980E1; Wed, 21 Aug 2019 17:32:05 +0000 (UTC) Received: from localhost (unused-10-15-17-196.yyz.redhat.com [10.15.17.196]) by smtp.corp.redhat.com (Postfix) with ESMTP id D47EA7E43; Wed, 21 Aug 2019 17:32:04 +0000 (UTC) From: Sergio Durigan Junior To: "Christian Biesinger via gdb-patches" Cc: Christian Biesinger Subject: Re: [PATCH 3/3] Load system gdbinit files from a directory References: <20190820221745.147370-1-cbiesinger@google.com> <20190820221745.147370-4-cbiesinger@google.com> Date: Wed, 21 Aug 2019 17:32:00 -0000 In-Reply-To: <20190820221745.147370-4-cbiesinger@google.com> (Christian Biesinger via gdb-patches's message of "Tue, 20 Aug 2019 17:17:45 -0500") Message-ID: <87mug2jugb.fsf@redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.2 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-IsSubscribed: yes X-SW-Source: 2019-08/txt/msg00494.txt.bz2 On Tuesday, August 20 2019, Christian Biesinger via gdb-patches wrote: > gdb/ChangeLog: > > 2019-08-20 Christian Biesinger > > * config.in: Add SYSTEM_GDBINIT_DIR. config.in is also regenerated. > * configure: Regenerate. > * configure.ac: Add new option --with-system-gdbinit-dir. > * main.c (get_init_files): Change system_gdbinit argument to > a vector and return the files in SYSTEM_GDBINIT_DIR in > addition to SYSTEM_GDBINIT. > (captured_main_1): Update. > (print_gdb_help): Update. You'll need a descriptive commit log. > --- > gdb/config.in | 3 ++ > gdb/configure | 77 ++++++++++++++++++++++++++++++++++++++++++++---- > gdb/configure.ac | 3 ++ > gdb/main.c | 53 +++++++++++++++++++++++++++------ > 4 files changed, 121 insertions(+), 15 deletions(-) > > diff --git a/gdb/config.in b/gdb/config.in > index 26ca02f6a3..ca523fe4ab 100644 > --- a/gdb/config.in > +++ b/gdb/config.in > @@ -684,6 +684,9 @@ > /* automatically load a system-wide gdbinit file */ > #undef SYSTEM_GDBINIT > > +/* automatically load system-wide gdbinit files from this dir */ > +#undef SYSTEM_GDBINIT_DIR > + > /* Define if the system-gdbinit directory should be relocated when GDB is > moved. */ > #undef SYSTEM_GDBINIT_RELOCATABLE > diff --git a/gdb/configure b/gdb/configure > index cb71bbf057..e5aa2e6b3b 100755 > --- a/gdb/configure > +++ b/gdb/configure > @@ -693,6 +693,7 @@ WIN32LIBS > SER_HARDWIRE > WERROR_CFLAGS > WARN_CFLAGS > +SYSTEM_GDBINIT_DIR > SYSTEM_GDBINIT > TARGET_SYSTEM_ROOT > CONFIG_LDFLAGS > @@ -824,6 +825,7 @@ infodir > docdir > oldincludedir > includedir > +runstatedir > localstatedir > sharedstatedir > sysconfdir > @@ -884,6 +886,7 @@ with_libipt_prefix > with_included_regex > with_sysroot > with_system_gdbinit > +with_system_gdbinit_dir > enable_werror > enable_build_warnings > enable_gdb_build_warnings > @@ -956,6 +959,7 @@ datadir='${datarootdir}' > sysconfdir='${prefix}/etc' > sharedstatedir='${prefix}/com' > localstatedir='${prefix}/var' > +runstatedir='${localstatedir}/run' > includedir='${prefix}/include' > oldincludedir='/usr/include' > docdir='${datarootdir}/doc/${PACKAGE}' > @@ -1208,6 +1212,15 @@ do > | -silent | --silent | --silen | --sile | --sil) > silent=yes ;; > > + -runstatedir | --runstatedir | --runstatedi | --runstated \ > + | --runstate | --runstat | --runsta | --runst | --runs \ > + | --run | --ru | --r) > + ac_prev=runstatedir ;; > + -runstatedir=* | --runstatedir=* | --runstatedi=* | --runstated=* \ > + | --runstate=* | --runstat=* | --runsta=* | --runst=* | --runs=* \ > + | --run=* | --ru=* | --r=*) > + runstatedir=$ac_optarg ;; > + > -sbindir | --sbindir | --sbindi | --sbind | --sbin | --sbi | --sb) > ac_prev=sbindir ;; > -sbindir=* | --sbindir=* | --sbindi=* | --sbind=* | --sbin=* \ > @@ -1345,7 +1358,7 @@ fi > for ac_var in exec_prefix prefix bindir sbindir libexecdir datarootdir \ > datadir sysconfdir sharedstatedir localstatedir includedir \ > oldincludedir docdir infodir htmldir dvidir pdfdir psdir \ > - libdir localedir mandir > + libdir localedir mandir runstatedir > do > eval ac_val=\$$ac_var > # Remove trailing slashes. > @@ -1498,6 +1511,7 @@ Fine tuning of the installation directories: > --sysconfdir=DIR read-only single-machine data [PREFIX/etc] > --sharedstatedir=DIR modifiable architecture-independent data [PREFIX/com] > --localstatedir=DIR modifiable single-machine data [PREFIX/var] > + --runstatedir=DIR modifiable per-process data [LOCALSTATEDIR/run] > --libdir=DIR object code libraries [EPREFIX/lib] > --includedir=DIR C header files [PREFIX/include] > --oldincludedir=DIR C header files for non-gcc [/usr/include] > @@ -1618,6 +1632,9 @@ Optional Packages: > --with-sysroot[=DIR] search for usr/lib et al within DIR > --with-system-gdbinit=PATH > automatically load a system-wide gdbinit file > + --with-system-gdbinit-dir=PATH > + automatically load system-wide gdbinit files from > + this directory > --with-lzma support lzma compression (auto/yes/no) > --with-liblzma-prefix[=DIR] search for liblzma in DIR/include and DIR/lib > --without-liblzma-prefix don't search for liblzma in includedir and libdir > @@ -4683,7 +4700,7 @@ else > We can't simply define LARGE_OFF_T to be 9223372036854775807, > since some C++ compilers masquerading as C compilers > incorrectly reject 9223372036854775807. */ > -#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62)) > +#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31)) > int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721 > && LARGE_OFF_T % 2147483647 == 1) > ? 1 : -1]; > @@ -4729,7 +4746,7 @@ else > We can't simply define LARGE_OFF_T to be 9223372036854775807, > since some C++ compilers masquerading as C compilers > incorrectly reject 9223372036854775807. */ > -#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62)) > +#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31)) > int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721 > && LARGE_OFF_T % 2147483647 == 1) > ? 1 : -1]; > @@ -4753,7 +4770,7 @@ rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext > We can't simply define LARGE_OFF_T to be 9223372036854775807, > since some C++ compilers masquerading as C compilers > incorrectly reject 9223372036854775807. */ > -#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62)) > +#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31)) > int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721 > && LARGE_OFF_T % 2147483647 == 1) > ? 1 : -1]; > @@ -4798,7 +4815,7 @@ else > We can't simply define LARGE_OFF_T to be 9223372036854775807, > since some C++ compilers masquerading as C compilers > incorrectly reject 9223372036854775807. */ > -#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62)) > +#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31)) > int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721 > && LARGE_OFF_T % 2147483647 == 1) > ? 1 : -1]; > @@ -4822,7 +4839,7 @@ rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext > We can't simply define LARGE_OFF_T to be 9223372036854775807, > since some C++ compilers masquerading as C compilers > incorrectly reject 9223372036854775807. */ > -#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62)) > +#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31)) > int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721 > && LARGE_OFF_T % 2147483647 == 1) > ? 1 : -1]; > @@ -12872,6 +12889,8 @@ main () > if (*(data + i) != *(data3 + i)) > return 14; > close (fd); > + free (data); > + free (data3); > return 0; > } > _ACEOF > @@ -15239,6 +15258,52 @@ _ACEOF > > > > +# Check whether --with-system-gdbinit-dir was given. > +if test "${with_system_gdbinit_dir+set}" = set; then : > + withval=$with_system_gdbinit_dir; > + SYSTEM_GDBINIT_DIR=$withval > +else > + SYSTEM_GDBINIT_DIR= > +fi > + > + > + test "x$prefix" = xNONE && prefix="$ac_default_prefix" > + test "x$exec_prefix" = xNONE && exec_prefix='${prefix}' > + ac_define_dir=`eval echo $SYSTEM_GDBINIT_DIR` > + ac_define_dir=`eval echo $ac_define_dir` > + > +cat >>confdefs.h <<_ACEOF > +#define SYSTEM_GDBINIT_DIR "$ac_define_dir" > +_ACEOF > + > + > + > + > + if test "x$exec_prefix" = xNONE || test "x$exec_prefix" = 'x${prefix}'; then > + if test "x$prefix" = xNONE; then > + test_prefix=/usr/local > + else > + test_prefix=$prefix > + fi > + else > + test_prefix=$exec_prefix > + fi > + value=0 > + case ${ac_define_dir} in > + "${test_prefix}"|"${test_prefix}/"*|\ > + '${exec_prefix}'|'${exec_prefix}/'*) > + value=1 > + ;; > + esac > + > +cat >>confdefs.h <<_ACEOF > +#define SYSTEM_GDBINIT_DIR_RELOCATABLE $value > +_ACEOF > + > + > + > + > + > # Check whether --enable-werror was given. > if test "${enable_werror+set}" = set; then : > enableval=$enable_werror; case "${enableval}" in > diff --git a/gdb/configure.ac b/gdb/configure.ac > index 5a18c16405..27d67ead3e 100644 > --- a/gdb/configure.ac > +++ b/gdb/configure.ac > @@ -1844,6 +1844,9 @@ GDB_AC_DEFINE_RELOCATABLE(TARGET_SYSTEM_ROOT, sysroot, ${ac_define_dir}) > GDB_AC_WITH_DIR(SYSTEM_GDBINIT, system-gdbinit, > [automatically load a system-wide gdbinit file], > []) > +GDB_AC_WITH_DIR(SYSTEM_GDBINIT_DIR, system-gdbinit-dir, > + [automatically load system-wide gdbinit files from this directory], > + []) > > AM_GDB_WARNINGS > AM_GDB_UBSAN > diff --git a/gdb/main.c b/gdb/main.c > index a1d1904c9b..39957db4bd 100644 > --- a/gdb/main.c > +++ b/gdb/main.c > @@ -45,6 +45,7 @@ > #include "event-top.h" > #include "infrun.h" > #include "gdbsupport/signals-state-save-restore.h" > +#include > #include > #include "gdbsupport/pathstuff.h" > #include "cli/cli-style.h" > @@ -232,11 +233,11 @@ static std::string relocate_gdbinit_path_maybe_in_datadir (std::string file) > to be loaded, then SYSTEM_GDBINIT (resp. HOME_GDBINIT and > LOCAL_GDBINIT) is set to the empty string. */ > static void > -get_init_files (std::string *system_gdbinit, > +get_init_files (std::vector *system_gdbinit, > std::string *home_gdbinit, > std::string *local_gdbinit) > { > - static std::string sysgdbinit; > + static std::vector sysgdbinit; > static std::string homeinit; > static std::string localinit; > static int initialized = 0; > @@ -251,7 +252,28 @@ get_init_files (std::string *system_gdbinit, > relocate_gdbinit_path_maybe_in_datadir (SYSTEM_GDBINIT); > if (!relocated_sysgdbinit.empty () && > stat (relocated_sysgdbinit.c_str (), &s) == 0) > - sysgdbinit = relocated_sysgdbinit; > + sysgdbinit.push_back(relocated_sysgdbinit); > + } > + if (SYSTEM_GDBINIT_DIR[0]) > + { The indentation looks funny here. > + std::string relocated_gdbinit_dir = > + relocate_gdbinit_path_maybe_in_datadir (SYSTEM_GDBINIT_DIR); > + DIR* dir; > + if (!relocated_gdbinit_dir.empty () && > + (dir = opendir (relocated_gdbinit_dir.c_str ())) != nullptr) I may be wrong, but I think we discourage variable assignment inside conditions (but things are changing so fast with C++ that I don't know anymore!). > + { > + std::vector files; > + struct dirent* ent; > + while ((ent = readdir (dir)) != nullptr) Likewise. > + { > + if (ent->d_name[0] != '.') I think we should also check 'ent->d_type' and make sure it's DT_REG (in which case, the 'ent->d_name' check above could be removed). It's questionable whether we should also deal DT_LINK, but I don't think so (for now). > + files.push_back (relocated_gdbinit_dir + SLASH_STRING + > + ent->d_name); > + } > + closedir (dir); > + std::sort (files.begin (), files.end ()); > + sysgdbinit.insert (sysgdbinit.end (), files.begin (), files.end ()); > + } > } > > const char *homedir = getenv ("HOME"); > @@ -909,7 +931,8 @@ captured_main_1 (struct captured_main_args *context) > /* Lookup gdbinit files. Note that the gdbinit file name may be > overriden during file initialization, so get_init_files should be > called after gdb_init. */ > - std::string system_gdbinit, home_gdbinit, local_gdbinit; > + std::vector system_gdbinit; > + std::string home_gdbinit, local_gdbinit; > get_init_files (&system_gdbinit, &home_gdbinit, &local_gdbinit); > > /* Do these (and anything which might call wrap_here or *_filtered) > @@ -987,7 +1010,10 @@ captured_main_1 (struct captured_main_args *context) > processed; it sets global parameters, which are independent of > what file you are debugging or what directory you are in. */ > if (!system_gdbinit.empty () && !inhibit_gdbinit) > - ret = catch_command_errors (source_script, system_gdbinit.c_str (), 0); > + { > + for (const std::string& file : system_gdbinit) > + ret = catch_command_errors (source_script, file.c_str (), 0); > + } > > /* Read and execute $HOME/.gdbinit file, if it exists. This is done > *before* all the command line arguments are processed; it sets > @@ -1205,7 +1231,7 @@ gdb_main (struct captured_main_args *args) > static void > print_gdb_help (struct ui_file *stream) > { > - std::string system_gdbinit; > + std::vector system_gdbinit; > std::string home_gdbinit; > std::string local_gdbinit; > > @@ -1286,9 +1312,18 @@ Other options:\n\n\ > At startup, GDB reads the following init files and executes their commands:\n\ > "), stream); > if (!system_gdbinit.empty ()) > - fprintf_unfiltered (stream, _("\ > - * system-wide init file: %s\n\ > -"), system_gdbinit.c_str ()); > + { > + std::string output; > + for (size_t idx = 0; idx < system_gdbinit.size(); ++idx) Space between variable name and parens. > + { > + output += system_gdbinit[idx]; > + if (idx < system_gdbinit.size() - 1) > + output += ", "; > + } > + fprintf_unfiltered (stream, _("\ > + * system-wide init files: %s\n\ > +"), output.c_str ()); > + } > if (!home_gdbinit.empty ()) > fprintf_unfiltered (stream, _("\ > * user-specific init file: %s\n\ > -- > 2.23.0.rc1.153.gdeed80330f-goog This looks good, but it needs a testcase, documentation updates and a NEWS entry. Thanks, -- Sergio GPG key ID: 237A 54B1 0287 28BF 00EF 31F4 D0EB 7628 65FC 5E36 Please send encrypted e-mail if possible http://sergiodj.net/