diff mbox series

skip -gfile: call fnmatch without FNM_FILE_NAME

Message ID CAN30aBHVVT+G81i7WUtWkPBZUYutLiNsDGpvtXrBh__R3=TwKA@mail.gmail.com
State New
Headers
Series skip -gfile: call fnmatch without FNM_FILE_NAME |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 fail Patch failed to apply
linaro-tcwg-bot/tcwg_gdb_build--master-arm fail Patch failed to apply

Commit Message

Fangrui Song Dec. 30, 2024, 3:09 a.m. UTC 
  Sorry, sent wrongly to gdb@sourceware.org ...


---

fnmatch is called with the FNM_FILE_NAME flag so that `skip -gfi /usr/*`
doesn't match /usr/include/*.  This makes the file matching feature not
useful for STL headers that reside in multiple directories.  Drop the
FNM_FILE_NAME flag.
---
 gdb/doc/gdb.texinfo | 2 +-
 gdb/skip.c          | 4 ++--
 gdb/symtab.c        | 4 ++--
 gdb/utils.c         | 2 --
 4 files changed, 5 insertions(+), 7 deletions(-)


--
2.47.1

Comments

Eli Zaretskii Dec. 30, 2024, 1:19 p.m. UTC | #1 
> From: Fangrui Song <maskray@sourceware.org>
> Date: Sun, 29 Dec 2024 19:09:34 -0800
> 
> fnmatch is called with the FNM_FILE_NAME flag so that `skip -gfi /usr/*`
> doesn't match /usr/include/*.  This makes the file matching feature not
> useful for STL headers that reside in multiple directories.  Drop the
> FNM_FILE_NAME flag.
> ---
>  gdb/doc/gdb.texinfo | 2 +-
>  gdb/skip.c          | 4 ++--
>  gdb/symtab.c        | 4 ++--
>  gdb/utils.c         | 2 --
>  4 files changed, 5 insertions(+), 7 deletions(-)
> 
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index b985399cf34..7ae486e3b7a 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -6737,7 +6737,7 @@ Functions in @var{file} will be skipped over
> when stepping.
>  @itemx -gfi @var{file-glob-pattern}
>  @cindex skipping over files via glob-style patterns
>  Functions in files matching @var{file-glob-pattern} will be skipped
> -over when stepping.
> +over when stepping.  The metacharacter @code{*} matches slashes.

Thanks, but I think the sentence you added is less clear than it could
be, and readers might not understand what that means.  How about

  The directory separator character  wildcard character @file{*} 

> 
>  @smallexample
>  (@value{GDBP}) skip -gfi utils/*.c
> diff --git a/gdb/skip.c b/gdb/skip.c
> index 3791c29b1e0..082fd2edbc8 100644
> --- a/gdb/skip.c
> +++ b/gdb/skip.c
> @@ -531,7 +531,7 @@ skiplist_entry::do_skip_gfile_p (const
> symtab_and_line &function_sal) const
>    /* Check first sole SYMTAB->FILENAME.  It may not be a substring of
>       symtab_to_fullname as it may contain "./" etc.  */
>    if (gdb_filename_fnmatch (m_file.c_str (), function_sal.symtab->filename,
> -                           FNM_FILE_NAME | FNM_NOESCAPE) == 0)
> +                           FNM_NOESCAPE) == 0)
>      result = true;
> 
>    /* Before we invoke symtab_to_fullname, which is expensive, do a quick
> @@ -542,7 +542,7 @@ skiplist_entry::do_skip_gfile_p (const
> symtab_and_line &function_sal) const
>    else if (!basenames_may_differ
>        && gdb_filename_fnmatch (lbasename (m_file.c_str ()),
>                                lbasename (function_sal.symtab->filename),
> -                              FNM_FILE_NAME | FNM_NOESCAPE) != 0)
> +                              FNM_NOESCAPE) != 0)
>      result = false;
>    else
>      {
> diff --git a/gdb/symtab.c b/gdb/symtab.c
> index 106e540ca48..00c470b4d3e 100644
> --- a/gdb/symtab.c
> +++ b/gdb/symtab.c
> @@ -609,7 +609,7 @@ compare_glob_filenames_for_search (const char *filename,
>      {
>        return (search_path_elements == file_path_elements
>               && gdb_filename_fnmatch (search_name, filename,
> -                                      FNM_FILE_NAME | FNM_NOESCAPE) == 0);
> +                                      FNM_NOESCAPE) == 0);
>      }
> 
>    {
> @@ -618,7 +618,7 @@ compare_glob_filenames_for_search (const char *filename,
>                                      file_path_elements - search_path_elements);
> 
>      return gdb_filename_fnmatch (search_name, file_to_compare,
> -                                FNM_FILE_NAME | FNM_NOESCAPE) == 0;
> +                                FNM_NOESCAPE) == 0;
>    }
>  }
> 
> diff --git a/gdb/utils.c b/gdb/utils.c
> index 6f2055e299d..d39cbacba71 100644
> --- a/gdb/utils.c
> +++ b/gdb/utils.c
> @@ -3500,8 +3500,6 @@ wait_to_die_with_timeout (pid_t pid, int
> *status, int timeout)
>  int
>  gdb_filename_fnmatch (const char *pattern, const char *string, int flags)
>  {
> -  gdb_assert ((flags & FNM_FILE_NAME) != 0);
> -
>    /* It is unclear how '\' escaping vs. directory separator should coexist.  */
>    gdb_assert ((flags & FNM_NOESCAPE) != 0);
> 
> --
> 2.47.1
>
Eli Zaretskii Dec. 30, 2024, 1:22 p.m. UTC | #2 
> From: Fangrui Song <maskray@sourceware.org>
> Date: Sun, 29 Dec 2024 19:09:34 -0800
> 
> fnmatch is called with the FNM_FILE_NAME flag so that `skip -gfi /usr/*`
> doesn't match /usr/include/*.  This makes the file matching feature not
> useful for STL headers that reside in multiple directories.  Drop the
> FNM_FILE_NAME flag.
> ---
>  gdb/doc/gdb.texinfo | 2 +-
>  gdb/skip.c          | 4 ++--
>  gdb/symtab.c        | 4 ++--
>  gdb/utils.c         | 2 --
>  4 files changed, 5 insertions(+), 7 deletions(-)
> 
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index b985399cf34..7ae486e3b7a 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -6737,7 +6737,7 @@ Functions in @var{file} will be skipped over
> when stepping.
>  @itemx -gfi @var{file-glob-pattern}
>  @cindex skipping over files via glob-style patterns
>  Functions in files matching @var{file-glob-pattern} will be skipped
> -over when stepping.
> +over when stepping.  The metacharacter @code{*} matches slashes.

Thanks, but I think the sentence you added is less clear than it could
be, and readers might not understand what that means.  How about

  The directory separator character @file{/} is treated as a regular
  character, so it can be matched by wildcard characters @file{*} and
  @file{?}.

Reviewed-By: Eli Zaretskii <eliz@gnu.org>
diff mbox series

Patch

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index b985399cf34..7ae486e3b7a 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -6737,7 +6737,7 @@  Functions in @var{file} will be skipped over
when stepping.
 @itemx -gfi @var{file-glob-pattern}
 @cindex skipping over files via glob-style patterns
 Functions in files matching @var{file-glob-pattern} will be skipped
-over when stepping.
+over when stepping.  The metacharacter @code{*} matches slashes.

 @smallexample
 (@value{GDBP}) skip -gfi utils/*.c
diff --git a/gdb/skip.c b/gdb/skip.c
index 3791c29b1e0..082fd2edbc8 100644
--- a/gdb/skip.c
+++ b/gdb/skip.c
@@ -531,7 +531,7 @@  skiplist_entry::do_skip_gfile_p (const
symtab_and_line &function_sal) const
   /* Check first sole SYMTAB->FILENAME.  It may not be a substring of
      symtab_to_fullname as it may contain "./" etc.  */
   if (gdb_filename_fnmatch (m_file.c_str (), function_sal.symtab->filename,
-                           FNM_FILE_NAME | FNM_NOESCAPE) == 0)
+                           FNM_NOESCAPE) == 0)
     result = true;

   /* Before we invoke symtab_to_fullname, which is expensive, do a quick
@@ -542,7 +542,7 @@  skiplist_entry::do_skip_gfile_p (const
symtab_and_line &function_sal) const
   else if (!basenames_may_differ
       && gdb_filename_fnmatch (lbasename (m_file.c_str ()),
                               lbasename (function_sal.symtab->filename),
-                              FNM_FILE_NAME | FNM_NOESCAPE) != 0)
+                              FNM_NOESCAPE) != 0)
     result = false;
   else
     {
diff --git a/gdb/symtab.c b/gdb/symtab.c
index 106e540ca48..00c470b4d3e 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -609,7 +609,7 @@  compare_glob_filenames_for_search (const char *filename,
     {
       return (search_path_elements == file_path_elements
              && gdb_filename_fnmatch (search_name, filename,
-                                      FNM_FILE_NAME | FNM_NOESCAPE) == 0);
+                                      FNM_NOESCAPE) == 0);
     }

   {
@@ -618,7 +618,7 @@  compare_glob_filenames_for_search (const char *filename,
                                     file_path_elements - search_path_elements);

     return gdb_filename_fnmatch (search_name, file_to_compare,
-                                FNM_FILE_NAME | FNM_NOESCAPE) == 0;
+                                FNM_NOESCAPE) == 0;
   }
 }

diff --git a/gdb/utils.c b/gdb/utils.c
index 6f2055e299d..d39cbacba71 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -3500,8 +3500,6 @@  wait_to_die_with_timeout (pid_t pid, int
*status, int timeout)
 int
 gdb_filename_fnmatch (const char *pattern, const char *string, int flags)
 {
-  gdb_assert ((flags & FNM_FILE_NAME) != 0);
-
   /* It is unclear how '\' escaping vs. directory separator should coexist.  */
   gdb_assert ((flags & FNM_NOESCAPE) != 0);