bugfindutils - Bugs: bug #62227, Incorrect warning for -name /

 
 

bug #62227: Incorrect warning for -name /

Submitter:  None
Submitted:  Mon 28 Mar 2022 06:37:13 AM UTC
 
Category:  find Severity:  3 - Normal
Item Group:  Wrong result Status:  Fixed
Privacy:  Public Assigned to:  berny
Originator Name:  Originator Email:  -email is unavailable-
Open/Closed:  Open Release:  4.9.0
Fixed Release:  None
* Mandatory Fields

Add a New Comment (Rich Markup)
   

Comment Type & Canned Response:
       

 

( Jump to the original submission )

Sun 24 Apr 2022 11:50:43 AM UTC, comment #7: 

Good catch, thanks.

Fixed and pushed here:
https://git.sv.gnu.org/cgit/findutils.git/commit/?id=6d2fa2c9

Bernhard Voelker <berny>
Project Administrator
Fri 15 Apr 2022 07:14:24 AM UTC, comment #6: 

comment #5:

> Okay, I agree.
>
> The attached patch changes the behavior (with the first one doing
> a trivial cleanup), and I'd like to improve the documentation further in a separate patch.
>
> Please check.
>
> (file #53051, file #53052)


Thanks, the code seems to do what I expect. However, the test may fail if the test suite is run when stdin is not a tty, as that suppresses warnings. To test that, you can run 'make check </dev/null'.

Anonymous
Thu 07 Apr 2022 04:31:00 PM UTC, comment #5: 

Okay, I agree.

The attached patch changes the behavior (with the first one doing
a trivial cleanup), and I'd like to improve the documentation further in a separate patch.

Please check.

(file #53051, file #53052)

Bernhard Voelker <berny>
Project Administrator
Tue 29 Mar 2022 01:04:41 PM UTC, comment #4: 

comment #3:

> So for my understanding:
> if 'find / -prune -name /' boils down to 'echo /', then what is
> the real use case behind?


In that case it does not do anything useful, sure. To get something useful we would need to add more. A minimal example to demonstrate the use of -name / might be

$ find / ! -name / -prune

This prints all entries in the root directory without recursing, in a way that should work in any POSIX-compatible implementation of find.

Anonymous
Tue 29 Mar 2022 06:14:58 AM UTC, comment #3: 

I'm just a bit reluctant because '-name /' will really only find
a match if "/" is given as starting point (which we don't know at
the time of parsing the expression tree).
In all other cases it is better for a user to keep the simple rule
in mind that the -name pattern must never contain a slash.
Hence to issue a warning is a good thing IMO.

So for my understanding:
if 'find / -prune -name /' boils down to 'echo /', then what is
the real use case behind?

Bernhard Voelker <berny>
Project Administrator
Tue 29 Mar 2022 01:41:22 AM UTC, comment #2: 

comment #1:

> I personally like to get such a warning, as one should try to use -name/-iname
> with patterns for basenames only, and I think that the use case 'find / -prune -name /'
> is quite exotic (and I would never have tried it myself TBH), so I'm wondering if it's
> worth bothering to improve the warning diagnostic as shown above.


I agree that it makes sense to warn for a name that cannot ever be a basename, but that means no warning should be issued for -name /, as / is a valid basename.

For the record, -name / and -wholename / do not match in exactly the same cases, because the basename of the root directory is / regardless of how it is spelt:

$ find /// -prune -name /
find: warning: ‘-name’ matches against basenames only, but the given pattern contains a directory separator (‘/’), thus the expression will evaluate to false all the time.  Did you mean ‘-wholename’?
///

$ find /// -prune -wholename /
find: warning: -wholename / will not match anything because it ends with /.

(That is another incorrect warning, -wholename / certainly can match despite ending in /, but that is not part of this bug report.)

Anonymous
Mon 28 Mar 2022 09:24:14 PM UTC, comment #1: 

It's true: even a comment in function pred_name_common [1] says that the
name pattern '/' has to match the "/" root file system entry:

Recall that 'find / -name /' is one of the few times where a '/'
in the -name must actually find something.

[1] https://git.savannah.gnu.org/cgit/findutils.git/tree/find/pred.c?id=43679658e2b#n464

The check with the warning is a GNU extension during the parsing of the command line
in function check_name_arg [2], and is therefore not issued with either the -nowarn
option or the environment variable POSIXLY_CORRECT set.

[2] https://git.savannah.gnu.org/cgit/findutils.git/tree/find/parser.c?id=43679658e2b#n1270

Well, we could differentiate if the pattern is a plain "/" and in that
case say something like this:

diff --git a/find/parser.c b/find/parser.c
index c65ae1a8..0562372f 100644
--- a/find/parser.c
+++ b/find/parser.c
@@ -1272,14 +1272,29 @@ check_name_arg (const char *pred, const char *alt, const char *arg)
 {
   if (should_issue_warnings () && strchr (arg, '/'))
     {
-      error (0, 0,
-            _("warning: %s matches against basenames only, "
-              "but the given pattern contains a directory separator (%s), "
-              "thus the expression will evaluate to false all the time.  "
-              "Did you mean %s?"),
-           safely_quote_err_filename (0, pred),
-           safely_quote_err_filename (1, "/"),
-           safely_quote_err_filename (2, alt));
+      if (0 == strcmp ("/", arg))
+       {
+         error (0, 0,
+                _("warning: %s matches against basenames only, "
+                  "but the given pattern equals the directory separator (%s), "
+                  "and hence will only match the file system root entry %s.  "
+                  "Did you mean %s?"),
+                safely_quote_err_filename (0, pred),
+                safely_quote_err_filename (1, "/"),
+                safely_quote_err_filename (2, "/"),
+                safely_quote_err_filename (3, alt));
+       }
+      else
+       {
+         error (0, 0,
+                _("warning: %s matches against basenames only, "
+                  "but the given pattern contains a directory separator (%s), "
+                  "thus the expression will evaluate to false all the time.  "
+                  "Did you mean %s?"),
+                safely_quote_err_filename (0, pred),
+                safely_quote_err_filename (1, "/"),
+                safely_quote_err_filename (2, alt));
+       }
     }
 }

This would result in:

find: warning: '-name' matches against basenames only, but the given pattern equals the directory separator ('/'), and hence will only match the file system root entry '/'.  Did you mean '-wholename'?

I personally like to get such a warning, as one should try to use -name/-iname
with patterns for basenames only, and I think that the use case 'find / -prune -name /'
is quite exotic (and I would never have tried it myself TBH), so I'm wondering if it's
worth bothering to improve the warning diagnostic as shown above.

Bernhard Voelker <berny>
Project Administrator
Mon 28 Mar 2022 06:37:13 AM UTC, original submission:  

$ find / -prune -name /
find/find: warning: ‘-name’ matches against basenames only, but the given pattern contains a directory separator (‘/’), thus the expression will evaluate to false all the time.  Did you mean ‘-wholename’?
/

The warning says -name / will evaluate to false all the time, but as the test demonstrates by printing /, in this case it is not correct. The warning makes sense for -name options where the argument to -name is multiple characters and one of them is /, but when the full argument is /, there should probably be no warning at all, or if you do feel it should still warn, the warning text should be changed to something that is more accurate.

Anonymous

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #53051:  0001-maint-simplify-check_name_arg-in-parser.c.patch added by berny (3KiB - text/x-patch - PATCH: find: omit warning diagnostic for -name '/')
file #53052:  0002-find-omit-warning-diagnostic-for-name.patch added by berny (7KiB - text/x-patch - PATCH: find: omit warning diagnostic for -name '/')

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by berny (Posted a comment)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only logged-in users can vote.

     

     

     

     

    Follow 6 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2022-04-24 berny StatusCode Review Fixed
    2022-04-07 berny Attached File- Added 0001-maint-simplify-check_name_arg-in-parser.c.patch, #53051
        Attached File- Added 0002-find-omit-warning-diagnostic-for-name.patch, #53052
        StatusIn Progress Code Review
    2022-03-28 berny StatusNone In Progress
        Assigned toNone berny

    Back to the top

    Powered by Savane 3.10