* [PATCH] gdb: convert auto-load to new-style debug macros
@ 2021-01-21 3:40 Simon Marchi via Gdb-patches
2021-01-21 19:05 ` Tom Tromey
0 siblings, 1 reply; 3+ messages in thread
From: Simon Marchi via Gdb-patches @ 2021-01-21 3:40 UTC (permalink / raw)
To: gdb-patches
Function file_is_auto_load_safe was taking a format string and varargs
just to output a debug print. This is probably because that function is
used in linux-thread-db.c and main.c, but debug_auto_load is static in
auto-load.c. I simplified that, making debug_auto_load visible outside
of auto-load.c, and making the callers of file_is_auto_load_safe output
the debug print themselves.
The rest is pretty much standard.
gdb/ChangeLog:
* auto-load.h (debug_auto_load): Move here.
(auto_load_debug_printf): New.
* auto-load.c: Use auto_load_debug_printf.
(debug_auto_load): Move to header.
* linux-thread-db.c (try_thread_db_load): Use
auto_load_debug_printf.
* main.c (captured_main_1): Likewise.
Change-Id: I468dc2a1d24b7dbf171f55181a11abbfafe70ba1
---
gdb/auto-load.c | 146 +++++++++++++++++-------------------------
gdb/auto-load.h | 13 +++-
gdb/linux-thread-db.c | 9 +--
gdb/main.c | 16 +++--
4 files changed, 82 insertions(+), 102 deletions(-)
diff --git a/gdb/auto-load.c b/gdb/auto-load.c
index 522f43b704cb..1dfcf21eeebf 100644
--- a/gdb/auto-load.c
+++ b/gdb/auto-load.c
@@ -61,8 +61,9 @@ static void maybe_print_script_not_found_warning
const struct extension_language_defn *language,
const char *section_name, unsigned offset);
-/* Value of the 'set debug auto-load' configuration variable. */
-static bool debug_auto_load = false;
+/* See auto-load.h. */
+
+bool debug_auto_load = false;
/* "show" command for the debug_auto_load configuration variable. */
@@ -183,8 +184,7 @@ auto_load_expand_dir_vars (const char *string)
substitute_path_component (&s, "$debugdir", debug_file_directory);
if (debug_auto_load && strcmp (s, string) != 0)
- fprintf_unfiltered (gdb_stdlog,
- _("auto-load: Expanded $-variables to \"%s\".\n"), s);
+ auto_load_debug_printf ("Expanded $-variables to \"%s\".", s);
std::vector<gdb::unique_xmalloc_ptr<char>> dir_vec
= dirnames_to_char_ptr_vec (s);
@@ -198,10 +198,8 @@ auto_load_expand_dir_vars (const char *string)
static void
auto_load_safe_path_vec_update (void)
{
- if (debug_auto_load)
- fprintf_unfiltered (gdb_stdlog,
- _("auto-load: Updating directories of \"%s\".\n"),
- auto_load_safe_path);
+ auto_load_debug_printf ("Updating directories of \"%s\".",
+ auto_load_safe_path);
auto_load_safe_path_vec = auto_load_expand_dir_vars (auto_load_safe_path);
size_t len = auto_load_safe_path_vec.size ();
@@ -222,23 +220,18 @@ auto_load_safe_path_vec_update (void)
if (debug_auto_load)
{
if (strcmp (in_vec.get (), original.get ()) == 0)
- fprintf_unfiltered (gdb_stdlog,
- _("auto-load: Using directory \"%s\".\n"),
- in_vec.get ());
+ auto_load_debug_printf ("Using directory \"%s\".",
+ in_vec.get ());
else
- fprintf_unfiltered (gdb_stdlog,
- _("auto-load: Resolved directory \"%s\" "
- "as \"%s\".\n"),
- original.get (), in_vec.get ());
+ auto_load_debug_printf ("Resolved directory \"%s\" as \"%s\".",
+ original.get (), in_vec.get ());
}
/* If gdb_realpath returns a different content, append it. */
if (strcmp (real_path.get (), in_vec.get ()) != 0)
{
- if (debug_auto_load)
- fprintf_unfiltered (gdb_stdlog,
- _("auto-load: And canonicalized as \"%s\".\n"),
- real_path.get ());
+ auto_load_debug_printf ("And canonicalized as \"%s\".",
+ real_path.get ());
auto_load_safe_path_vec.push_back (std::move (real_path));
}
@@ -338,10 +331,8 @@ filename_is_in_pattern_1 (char *filename, char *pattern)
size_t pattern_len = strlen (pattern);
size_t filename_len = strlen (filename);
- if (debug_auto_load)
- fprintf_unfiltered (gdb_stdlog, _("auto-load: Matching file \"%s\" "
- "to pattern \"%s\"\n"),
- filename, pattern);
+ auto_load_debug_printf ("Matching file \"%s\" to pattern \"%s\"",
+ filename, pattern);
/* Trim trailing slashes ("/") from PATTERN. Even for "d:\" paths as
trailing slashes are trimmed also from FILENAME it still matches
@@ -355,9 +346,7 @@ filename_is_in_pattern_1 (char *filename, char *pattern)
IS_DIR_SEPARATOR character, such as the 'C:\x.exe' filename. */
if (pattern_len == 0)
{
- if (debug_auto_load)
- fprintf_unfiltered (gdb_stdlog,
- _("auto-load: Matched - empty pattern\n"));
+ auto_load_debug_printf ("Matched - empty pattern");
return 1;
}
@@ -370,20 +359,15 @@ filename_is_in_pattern_1 (char *filename, char *pattern)
filename[filename_len] = '\0';
if (filename_len == 0)
{
- if (debug_auto_load)
- fprintf_unfiltered (gdb_stdlog,
- _("auto-load: Not matched - pattern \"%s\".\n"),
- pattern);
+ auto_load_debug_printf ("Not matched - pattern \"%s\".", pattern);
return 0;
}
if (gdb_filename_fnmatch (pattern, filename, FNM_FILE_NAME | FNM_NOESCAPE)
== 0)
{
- if (debug_auto_load)
- fprintf_unfiltered (gdb_stdlog, _("auto-load: Matched - file "
- "\"%s\" to pattern \"%s\".\n"),
- filename, pattern);
+ auto_load_debug_printf ("Matched - file \"%s\" to pattern \"%s\".",
+ filename, pattern);
return 1;
}
@@ -434,10 +418,8 @@ filename_is_in_auto_load_safe_path_vec (const char *filename,
{
*filename_realp = gdb_realpath (filename);
if (debug_auto_load && strcmp (filename_realp->get (), filename) != 0)
- fprintf_unfiltered (gdb_stdlog,
- _("auto-load: Resolved "
- "file \"%s\" as \"%s\".\n"),
- filename, filename_realp->get ());
+ auto_load_debug_printf ("Resolved file \"%s\" as \"%s\".",
+ filename, filename_realp->get ());
}
if (strcmp (filename_realp->get (), filename) != 0)
@@ -451,10 +433,8 @@ filename_is_in_auto_load_safe_path_vec (const char *filename,
if (pattern != NULL)
{
- if (debug_auto_load)
- fprintf_unfiltered (gdb_stdlog, _("auto-load: File \"%s\" matches "
- "directory \"%s\".\n"),
- filename, pattern);
+ auto_load_debug_printf ("File \"%s\" matches directory \"%s\".",
+ filename, pattern);
return 1;
}
@@ -464,20 +444,11 @@ filename_is_in_auto_load_safe_path_vec (const char *filename,
/* See auto-load.h. */
bool
-file_is_auto_load_safe (const char *filename, const char *debug_fmt, ...)
+file_is_auto_load_safe (const char *filename)
{
gdb::unique_xmalloc_ptr<char> filename_real;
static bool advice_printed = false;
- if (debug_auto_load)
- {
- va_list debug_args;
-
- va_start (debug_args, debug_fmt);
- vfprintf_unfiltered (gdb_stdlog, debug_fmt, debug_args);
- va_end (debug_args);
- }
-
if (filename_is_in_auto_load_safe_path_vec (filename, &filename_real))
return true;
@@ -766,9 +737,10 @@ auto_load_objfile_script_1 (struct objfile *objfile, const char *realname,
gdb_file_up input = gdb_fopen_cloexec (filename.c_str (), "r");
debugfile = filename.c_str ();
- if (debug_auto_load)
- fprintf_unfiltered (gdb_stdlog, _("auto-load: Attempted file \"%s\" %s.\n"),
- debugfile, input ? _("exists") : _("does not exist"));
+
+ auto_load_debug_printf ("Attempted file \"%s\" %s.",
+ debugfile,
+ input != nullptr ? "exists" : "does not exist");
std::string debugfile_holder;
if (!input)
@@ -779,10 +751,9 @@ auto_load_objfile_script_1 (struct objfile *objfile, const char *realname,
std::vector<gdb::unique_xmalloc_ptr<char>> vec
= auto_load_expand_dir_vars (auto_load_dir);
- if (debug_auto_load)
- fprintf_unfiltered (gdb_stdlog, _("auto-load: Searching 'set auto-load "
- "scripts-directory' path \"%s\".\n"),
- auto_load_dir);
+ auto_load_debug_printf
+ ("Searching 'set auto-load scripts-directory' path \"%s\".",
+ auto_load_dir);
/* Convert Windows file name from c:/dir/file to /c/dir/file. */
if (HAS_DRIVE_SPEC (debugfile))
@@ -796,11 +767,13 @@ auto_load_objfile_script_1 (struct objfile *objfile, const char *realname,
debugfile = debugfile_holder.c_str ();
input = gdb_fopen_cloexec (debugfile, "r");
- if (debug_auto_load)
- fprintf_unfiltered (gdb_stdlog, _("auto-load: Attempted file "
- "\"%s\" %s.\n"),
- debugfile,
- input ? _("exists") : _("does not exist"));
+
+ auto_load_debug_printf ("Attempted file \"%s\" %s.",
+ debugfile,
+ (input != nullptr
+ ? "exists"
+ : "does not exist"));
+
if (input != NULL)
break;
}
@@ -810,12 +783,11 @@ auto_load_objfile_script_1 (struct objfile *objfile, const char *realname,
{
struct auto_load_pspace_info *pspace_info;
- bool is_safe
- = file_is_auto_load_safe (debugfile,
- _("auto-load: Loading %s script \"%s\""
- " by extension for objfile \"%s\".\n"),
- ext_lang_name (language),
- debugfile, objfile_name (objfile));
+ auto_load_debug_printf
+ ("Loading %s script \"%s\" by extension for objfile \"%s\".",
+ ext_lang_name (language), debugfile, objfile_name (objfile));
+
+ bool is_safe = file_is_auto_load_safe (debugfile);
/* Add this script to the hash table too so
"info auto-load ${lang}-scripts" can print it. */
@@ -871,10 +843,11 @@ auto_load_objfile_script (struct objfile *objfile,
{
len -= lexe;
realname.get ()[len] = '\0';
- if (debug_auto_load)
- fprintf_unfiltered (gdb_stdlog, _("auto-load: Stripped .exe suffix, "
- "retrying with \"%s\".\n"),
- realname.get ());
+
+ auto_load_debug_printf
+ ("auto-load: Stripped .exe suffix, retrying with \"%s\".",
+ realname.get ());
+
auto_load_objfile_script_1 (objfile, realname.get (), language);
}
}
@@ -917,13 +890,12 @@ source_script_file (struct auto_load_pspace_info *pspace_info,
if (opened)
{
- if (!file_is_auto_load_safe (opened->full_path.get (),
- _("auto-load: Loading %s script "
- "\"%s\" from section \"%s\" of "
- "objfile \"%s\".\n"),
- ext_lang_name (language),
- opened->full_path.get (),
- section_name, objfile_name (objfile)))
+ auto_load_debug_printf
+ ("Loading %s script \"%s\" from section \"%s\" of objfile \"%s\".",
+ ext_lang_name (language), opened->full_path.get (),
+ section_name, objfile_name (objfile));
+
+ if (!file_is_auto_load_safe (opened->full_path.get ()))
opened.reset ();
}
else
@@ -1018,13 +990,11 @@ of file %ps."),
return;
}
- bool is_safe
- = file_is_auto_load_safe (objfile_name (objfile),
- _("auto-load: Loading %s script "
- "\"%s\" from section \"%s\" of "
- "objfile \"%s\".\n"),
- ext_lang_name (language), name,
- section_name, objfile_name (objfile));
+ auto_load_debug_printf
+ ("Loading %s script \"%s\" from section \"%s\" of objfile \"%s\".",
+ ext_lang_name (language), name, section_name, objfile_name (objfile));
+
+ bool is_safe = file_is_auto_load_safe (objfile_name (objfile));
bool in_hash_table
= maybe_add_script_text (pspace_info, is_safe, name, language);
diff --git a/gdb/auto-load.h b/gdb/auto-load.h
index ec802395d3ed..f726126c5541 100644
--- a/gdb/auto-load.h
+++ b/gdb/auto-load.h
@@ -25,6 +25,15 @@ struct program_space;
struct auto_load_pspace_info;
struct extension_language_defn;
+/* Value of the 'set debug auto-load' configuration variable. */
+
+extern bool debug_auto_load;
+
+/* Print an "auto-load" debug statement. */
+
+#define auto_load_debug_printf(fmt, ...) \
+ debug_prefixed_printf_cond (debug_auto_load, "auto-load", fmt, ##__VA_ARGS__)
+
extern bool global_auto_load;
extern bool auto_load_local_gdbinit;
@@ -52,9 +61,7 @@ extern struct cmd_list_element **auto_load_info_cmdlist_get (void);
even if the caller would quietly skip non-existing file in unsafe
directory. */
-extern bool file_is_auto_load_safe (const char *filename,
- const char *debug_fmt, ...)
- ATTRIBUTE_PRINTF (2, 3);
+extern bool file_is_auto_load_safe (const char *filename);
/* Return true if auto-loading gdb scripts is enabled. */
diff --git a/gdb/linux-thread-db.c b/gdb/linux-thread-db.c
index 0a32e40f4d5f..dce4bd23c1b7 100644
--- a/gdb/linux-thread-db.c
+++ b/gdb/linux-thread-db.c
@@ -976,10 +976,11 @@ try_thread_db_load (const char *library, bool check_auto_load_safe)
return false;
}
- if (!file_is_auto_load_safe (library, _("auto-load: Loading libthread-db "
- "library \"%s\" from explicit "
- "directory.\n"),
- library))
+ auto_load_debug_printf
+ ("Loading libthread-db library \"%s\" from explicit directory.",
+ library);
+
+ if (!file_is_auto_load_safe (library))
return false;
}
diff --git a/gdb/main.c b/gdb/main.c
index 1e1fbf2f3512..331e3a50acf8 100644
--- a/gdb/main.c
+++ b/gdb/main.c
@@ -1185,15 +1185,17 @@ captured_main_1 (struct captured_main_args *context)
auto_load_local_gdbinit_pathname
= gdb_realpath (local_gdbinit.c_str ()).release ();
- if (!inhibit_gdbinit && auto_load_local_gdbinit
- && file_is_auto_load_safe (local_gdbinit.c_str (),
- _("auto-load: Loading .gdbinit "
- "file \"%s\".\n"),
- local_gdbinit.c_str ()))
+ if (!inhibit_gdbinit && auto_load_local_gdbinit)
{
- auto_load_local_gdbinit_loaded = 1;
+ auto_load_debug_printf ("Loading .gdbinit file \"%s\".",
+ local_gdbinit.c_str ());
- ret = catch_command_errors (source_script, local_gdbinit.c_str (), 0);
+ if (file_is_auto_load_safe (local_gdbinit.c_str ()))
+ {
+ auto_load_local_gdbinit_loaded = 1;
+
+ ret = catch_command_errors (source_script, local_gdbinit.c_str (), 0);
+ }
}
}
--
2.30.0
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] gdb: convert auto-load to new-style debug macros
2021-01-21 3:40 [PATCH] gdb: convert auto-load to new-style debug macros Simon Marchi via Gdb-patches
@ 2021-01-21 19:05 ` Tom Tromey
2021-01-21 19:11 ` Simon Marchi via Gdb-patches
0 siblings, 1 reply; 3+ messages in thread
From: Tom Tromey @ 2021-01-21 19:05 UTC (permalink / raw)
To: Simon Marchi via Gdb-patches
>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:
Simon> Function file_is_auto_load_safe was taking a format string and varargs
Simon> just to output a debug print. This is probably because that function is
Simon> used in linux-thread-db.c and main.c, but debug_auto_load is static in
Simon> auto-load.c. I simplified that, making debug_auto_load visible outside
Simon> of auto-load.c, and making the callers of file_is_auto_load_safe output
Simon> the debug print themselves.
Simon> The rest is pretty much standard.
Simon> gdb/ChangeLog:
Simon> * auto-load.h (debug_auto_load): Move here.
Simon> (auto_load_debug_printf): New.
Simon> * auto-load.c: Use auto_load_debug_printf.
Simon> (debug_auto_load): Move to header.
Simon> * linux-thread-db.c (try_thread_db_load): Use
Simon> auto_load_debug_printf.
Simon> * main.c (captured_main_1): Likewise.
Thanks. This seems like a nice cleanup.
I have one comment.
Simon> - _("auto-load: Loading .gdbinit "
Simon> - "file \"%s\".\n"),
Simon> - local_gdbinit.c_str ()))
Simon> + if (!inhibit_gdbinit && auto_load_local_gdbinit)
Simon> {
Simon> - auto_load_local_gdbinit_loaded = 1;
Simon> + auto_load_debug_printf ("Loading .gdbinit file \"%s\".",
Simon> + local_gdbinit.c_str ());
Some of these changes lose the _() wrapper.
I don't know if we care for debug logging .. or maybe we actively want
to remove them in this case? Anyway I thought I'd point it out.
Tom
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] gdb: convert auto-load to new-style debug macros
2021-01-21 19:05 ` Tom Tromey
@ 2021-01-21 19:11 ` Simon Marchi via Gdb-patches
0 siblings, 0 replies; 3+ messages in thread
From: Simon Marchi via Gdb-patches @ 2021-01-21 19:11 UTC (permalink / raw)
To: Tom Tromey, Simon Marchi via Gdb-patches
On 2021-01-21 2:05 p.m., Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:
>
> Simon> Function file_is_auto_load_safe was taking a format string and varargs
> Simon> just to output a debug print. This is probably because that function is
> Simon> used in linux-thread-db.c and main.c, but debug_auto_load is static in
> Simon> auto-load.c. I simplified that, making debug_auto_load visible outside
> Simon> of auto-load.c, and making the callers of file_is_auto_load_safe output
> Simon> the debug print themselves.
>
> Simon> The rest is pretty much standard.
>
> Simon> gdb/ChangeLog:
>
> Simon> * auto-load.h (debug_auto_load): Move here.
> Simon> (auto_load_debug_printf): New.
> Simon> * auto-load.c: Use auto_load_debug_printf.
> Simon> (debug_auto_load): Move to header.
> Simon> * linux-thread-db.c (try_thread_db_load): Use
> Simon> auto_load_debug_printf.
> Simon> * main.c (captured_main_1): Likewise.
>
> Thanks. This seems like a nice cleanup.
> I have one comment.
>
> Simon> - _("auto-load: Loading .gdbinit "
> Simon> - "file \"%s\".\n"),
> Simon> - local_gdbinit.c_str ()))
> Simon> + if (!inhibit_gdbinit && auto_load_local_gdbinit)
> Simon> {
> Simon> - auto_load_local_gdbinit_loaded = 1;
> Simon> + auto_load_debug_printf ("Loading .gdbinit file \"%s\".",
> Simon> + local_gdbinit.c_str ());
>
> Some of these changes lose the _() wrapper.
> I don't know if we care for debug logging .. or maybe we actively want
> to remove them in this case? Anyway I thought I'd point it out.
I removed them on purpose (will add a mention in the commit log), because
I have not seen them used for logging elsewhere. I don't really think
it's necessary.
I'll push with the mention added, and if I stumble on other places that
uses _ for logging, I'll remove it.
Thanks,
Simon
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-01-21 19:11 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-21 3:40 [PATCH] gdb: convert auto-load to new-style debug macros Simon Marchi via Gdb-patches
2021-01-21 19:05 ` Tom Tromey
2021-01-21 19:11 ` Simon Marchi via Gdb-patches
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox