Skip to content

bpo-38094: Remove unneeded assignment in PyBytes_Concat()#15274

Merged
gpshead merged 1 commit into
python:masterfrom
sir-sigurd:bytes-concat-len-1
Sep 10, 2019
Merged

bpo-38094: Remove unneeded assignment in PyBytes_Concat()#15274
gpshead merged 1 commit into
python:masterfrom
sir-sigurd:bytes-concat-len-1

Conversation

@sir-sigurd

@sir-sigurd sir-sigurd commented Aug 14, 2019 •
edited by bedevere-bot
Loading

Copy link
Copy Markdown
Contributor

It looks unneeded since its introduction in 161d695

https://bugs.python.org/issue38094

@sir-sigurd
Remove unneeded assignment in PyBytes_Concat()
6d3905b 
It looks unneeded since its introduction in
161d695
epicfaace
epicfaace reviewed Aug 27, 2019
View reviewed changes

@epicfaace epicfaace left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Isn't it used here:

cpython/Objects/bytesobject.c

Line 2949 in 6d3905b

if (oldsize > PY_SSIZE_T_MAX - wb.len) {

@sir-sigurd

sir-sigurd commented Aug 27, 2019

Copy link
Copy Markdown
Contributor Author

When wb.len is used it's set by PyObject_GetBuffer().

@gpshead gpshead changed the title Remove unneeded assignment in PyBytes_Concat() bpo-38094: Remove unneeded assignment in PyBytes_Concat() Sep 10, 2019
@gpshead

gpshead commented Sep 10, 2019

Copy link
Copy Markdown
Member

closing and reopening to re-trigger bedevere

@gpshead gpshead closed this Sep 10, 2019
@gpshead gpshead reopened this Sep 10, 2019
@gpshead gpshead self-assigned this Sep 10, 2019
@gpshead
gpshead merged commit afdeb18 into python:master Sep 10, 2019
@miss-islington

miss-islington commented Sep 10, 2019

Copy link
Copy Markdown
Contributor

Thanks @sir-sigurd for the PR, and @gpshead for merging it 🌮🎉.. I'm working now to backport this PR to: 3.8.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Sep 10, 2019
@sir-sigurd @miss-islington
Remove unneeded assignment in PyBytes_Concat() (pythonGH-15274)
43154ef 
The `wb.len = -1` assignment is unneeded since its introduction in 161d695 as `PyObject_GetBuffer` always fills it in.
(cherry picked from commit afdeb18)

Co-authored-by: Sergey Fedoseev <fedoseev.sergey@gmail.com>
@bedevere-bot

bedevere-bot commented Sep 10, 2019

Copy link
Copy Markdown

GH-15886 is a backport of this pull request to the 3.8 branch.

@miss-islington

miss-islington commented Sep 10, 2019

Copy link
Copy Markdown
Contributor

Thanks @sir-sigurd for the PR, and @gpshead for merging it 🌮🎉.. I'm working now to backport this PR to: 3.8.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Sep 10, 2019
@sir-sigurd @miss-islington
Remove unneeded assignment in PyBytes_Concat() (pythonGH-15274)
df755ea 
The `wb.len = -1` assignment is unneeded since its introduction in 161d695 as `PyObject_GetBuffer` always fills it in.
(cherry picked from commit afdeb18)

Co-authored-by: Sergey Fedoseev <fedoseev.sergey@gmail.com>
@bedevere-bot

bedevere-bot commented Sep 10, 2019

Copy link
Copy Markdown

GH-15887 is a backport of this pull request to the 3.8 branch.

@sir-sigurd
sir-sigurd deleted the bytes-concat-len-1 branch September 10, 2019 16:25
miss-islington added a commit that referenced this pull request Sep 10, 2019
@miss-islington @sir-sigurd
Remove unneeded assignment in PyBytes_Concat() (GH-15274)
98224d2 
The `wb.len = -1` assignment is unneeded since its introduction in 161d695 as `PyObject_GetBuffer` always fills it in.
(cherry picked from commit afdeb18)

Co-authored-by: Sergey Fedoseev <fedoseev.sergey@gmail.com>
websurfer5 pushed a commit to websurfer5/cpython that referenced this pull request Jul 20, 2020
@sir-sigurd @websurfer5
Remove unneeded assignment in PyBytes_Concat() (pythonGH-15274)
fecbec9 
The `wb.len = -1` assignment is unneeded since its introduction in 161d695 as `PyObject_GetBuffer` always fills it in.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gpshead gpshead gpshead approved these changes
+1 more reviewer
@epicfaace epicfaace epicfaace left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

@gpshead gpshead

Labels

skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@sir-sigurd @gpshead @miss-islington @bedevere-bot @epicfaace @the-knights-who-say-ni