Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Possible bad copy paste in glob.py #129350

Open
wooffie opened this issue Jan 27, 2025 · 5 comments
Open

Possible bad copy paste in glob.py #129350

wooffie opened this issue Jan 27, 2025 · 5 comments

Comments

@wooffie
Copy link
Contributor

wooffie commented Jan 27, 2025

I think it is bad copy paste here

yield pathname

Maybe we should yield dirname?

Additional information

Found by Linux Verification Center (linuxtesting.org) with SVACE.

Reporter: Burkov Egor (eburkov@rvision.ru).

Organization: R-Vision (support@rvision.ru).

Linked PRs

@StanFromIreland
Copy link
Contributor

StanFromIreland commented Jan 27, 2025

@serhiy-storchaka You created this, is this an error?

@encukou
Copy link
Member

encukou commented Jan 27, 2025

What is the problem here? What do you mean by "bad copy paste"?

This code was introduced in 6f20170, yield pathname has been working for a decade.

@serhiy-storchaka
Copy link
Member

It was here from the beginning, and it was intentional (the difference is in the trailing slash). Surprisingly, the tests were not failed after your change, because they are too lenient in this question for some reasons.

It is better to make the tests more strict so any unintentional changes will be caught.

serhiy-storchaka added a commit to serhiy-storchaka/cpython that referenced this issue Jan 27, 2025
Test that the trailing pathname separator is preserved.

Multiple trailing pathname separators are only preserved if the pattern
does not contain metacharacters, otherwise only one trailing pathname
separator is preserved. This is rather an implementation detail.
@wooffie
Copy link
Contributor Author

wooffie commented Jan 28, 2025

I thought about this logic, but can we transform:

if basename:
    if _lexists(_join(root_dir, pathname), dir_fd):
        yield pathname
else:
# Patterns ending with a slash should match only directories
    if _isdir(_join(root_dir, dirname), dir_fd):
         yield pathname
    return

Into:

if _lexists(_join(root_dir, pathname), dir_fd) or _isdir(_join(root_dir, dirname), dir_fd):
     yield pathname
return

@encukou
Copy link
Member

encukou commented Jan 28, 2025

No, that would match nonexistent files (and remove a helpful comment).

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants