Skip to content

gh-156264: add missing checks in os.chmod - #156265

Open
KowalskiThomas wants to merge 3 commits into
python:mainfrom
KowalskiThomas:kowalski/fix-add-missing-checks-in-os-chmod
Open

gh-156264: add missing checks in os.chmod#156265
KowalskiThomas wants to merge 3 commits into
python:mainfrom
KowalskiThomas:kowalski/fix-add-missing-checks-in-os-chmod

Conversation

@KowalskiThomas

@KowalskiThomas KowalskiThomas commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

@KowalskiThomas
KowalskiThomas force-pushed the kowalski/fix-add-missing-checks-in-os-chmod branch from f5c1ba3 to d40f8b5 Compare August 23, 2026 13:42
@KowalskiThomas
KowalskiThomas marked this pull request as ready for review August 23, 2026 20:01
Comment thread Modules/posixmodule.c Outdated
Comment on lines +4121 to +4122
if (fd_and_follow_symlinks_invalid("chmod", path->is_fd, follow_symlinks))
return NULL;

@picnixz picnixz Aug 24, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please follow PEP-7 here for clairty.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the comment!

I believe I fixed it all, namely:

  • I added curly braces to both new conditions
  • I split the line you commented on so that it's not more than 79 characters
  • I've also split the argument list such that it's one argument per line.
    • This one I don't think the rest of (at least) this file always agrees with but... I thought I'd follow what's expected by PEP-7.

@KowalskiThomas
KowalskiThomas requested a review from picnixz August 24, 2026 16:06
Comment thread Modules/posixmodule.c
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
@KowalskiThomas
KowalskiThomas requested a review from picnixz August 26, 2026 13:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

os.chmod is missing validation for dir_fd and follow_symlinks when path is a file descriptor

2 participants