classification
Title: PyBytes_FromObject(bytes_object) creates a new object
Type: behavior Stage: resolved
Components: Versions: Python 3.3
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: larry Nosy List: amaury.forgeotdarc, brett.cannon, hynek, jcea, larry, pitrou, python-dev, skrah
Priority: normal Keywords: patch

Created on 2012-05-23 14:39 by larry, last changed 2012-05-25 05:59 by larry. This issue is now closed.

Files
File name Uploaded Description Edit
larry.pybytes_fromobject.identity.1.diff larry, 2012-05-23 15:26 Patch adding an identity transformation to PyBytes_FromObject(). review
Messages (10)
msg161414 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2012-05-23 14:39
If you pass a valid PyUnicodeObject into PyUnicode_AsObject(), it incref's the original object and returns it.

If you pass a valid PyBytesObject into PyBytes_AsObject()... it fails.

I assert that in the PyBytes_AsObject() should behave like PyUnicode_AsObject() when faced with an identity transformation.

(Brett: I tagged you because I saw your name in the comments, so I figured you were a good candidate.)
msg161418 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2012-05-23 15:26
The appropriate four line patch.  (Six with whitespace.)
msg161420 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2012-05-23 15:29
I suppose you are referring to PyUnicode_FromObject() and PyBytes_FromObject()...
And by "it fails" did you simply mean "it fails to return the same object"?
msg161422 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2012-05-23 15:30
1) Yes, whoopsies.  It's late.

2) It fails, as in, it returns NULL.
msg161423 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2012-05-23 15:32
> 2) It fails, as in, it returns NULL.
It's not my experience. Do you have an example?
msg161424 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-05-23 15:34
Well, the patch is nice but the PyObject_CheckBuffer(...) part should have succeeded, so it's a bit mysterious why it doesn't.
msg161483 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2012-05-24 07:26
I also wonder how the buffer interface section can fail.
PyBuffer_ToContiguous() should translate to a simple memcpy()
for a bytes object. What is going on?
msg161485 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2012-05-24 08:06
I can't reproduce this, and it was pretty late in my day when I saw it, so let's assume I was mistaken and PyBytes_CheckBuffer() works fine.  Nevertheless I think the patch is a good idea--why create a new object when you don't have to?  Unless I hear otherwise I'll check it in in the next day or so.

As for what I saw, I suspect it was a deliberate TypeError from the regression test suite, passing in an integer 0 as a filename to os.rename().  Naturally PyBytes_FromObject() fails to convert that to a bytes object.
msg161551 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2012-05-25 05:41
Changing the name of the report to accurately reflect reality.  If you passed in a bytes object, PyBytes_FromObject would create a new object, when all it really needed to do was incref and return.  My checkin in a minute will add that shortcut.
msg161552 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2012-05-25 05:58
New changeset f8cd75e8a1d6 by Larry Hastings in branch 'default':
Issue #14889: PyBytes_FromObject(bytes) now just increfs and returns.
http://hg.python.org/cpython/rev/f8cd75e8a1d6
History
Date User Action Args
2012-05-25 05:59:22larrysetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2012-05-25 05:58:47python-devsetnosy: + python-dev
messages: + msg161552
2012-05-25 05:41:43larrysetmessages: + msg161551
title: PyBytes_FromObject(bytes_object) fails -> PyBytes_FromObject(bytes_object) creates a new object
2012-05-24 08:06:07larrysetmessages: + msg161485
2012-05-24 07:26:36skrahsetnosy: + skrah
messages: + msg161483
2012-05-23 16:05:57jceasetnosy: + jcea
2012-05-23 15:34:40pitrousetnosy: + pitrou
messages: + msg161424
2012-05-23 15:32:48amaury.forgeotdarcsetmessages: + msg161423
2012-05-23 15:30:33larrysetmessages: + msg161422
2012-05-23 15:29:14amaury.forgeotdarcsetnosy: + amaury.forgeotdarc
messages: + msg161420
2012-05-23 15:26:03larrysetfiles: + larry.pybytes_fromobject.identity.1.diff
keywords: + patch
messages: + msg161418

stage: needs patch -> patch review
2012-05-23 14:53:20hyneksetnosy: + hynek
2012-05-23 14:39:50larrycreate