Vlad Riscutia riscutiav...@gmail.com added the comment:
Changing title and type to better reflect issue.
On Windows MSVC build, ctypes is not correctly setting bitfields backed by 64
bit integers if specifying custom width. Simple repro:
from ctypes import *
class X(Structure):
_fields_
Vlad Riscutia riscutiav...@gmail.com added the comment:
Terry, I am kind of new to this so I might not have marked header correctly.
Please feel free to fix it.
This is 100% bug not feature request. Initial message was confusing because
highstar thought Python has such bitfields as readonly
Vlad Riscutia riscutiav...@gmail.com added the comment:
Attached is addition to unittests which repro the issue. They will currently
pass on Linux but fail on Windows.
--
keywords: +patch
Added file: http://bugs.python.org/file22447/issue6068_unittest.diff
Vlad Riscutia riscutiav...@gmail.com added the comment:
I took a look at this and I believe behavior is correct on Windows, the issue
is with the test. For example this test is failing:
class closest_fit(ctypes.BigEndianStructure):
_pack_ = 1# aligned to 8 bits, not ctypes default
Vlad Riscutia riscutiav...@gmail.com added the comment:
Attaching patch as previous attachment is only unittest. I included change to
SWAP_8 macro also as it looks like same issue and it will probably popup later.
--
Added file: http://bugs.python.org/file22451/issue6068_patch.diff
New submission from Vlad Riscutia riscutiav...@gmail.com:
Opened this issue to track configurable bitfield allocation strategy. This will
address issues like http://bugs.python.org/issue6069,
http://bugs.python.org/issue11920.
Summary: the way bitfields are allocated is up to the compiler
Vlad Riscutia riscutiav...@gmail.com added the comment:
Opened http://bugs.python.org/issue12528 to address this.
--
nosy: +vladris
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue11920
Vlad Riscutia riscutiav...@gmail.com added the comment:
Opened http://bugs.python.org/issue12528 to address this.
--
versions: +Python 3.3
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue6069
Vlad Riscutia riscutiav...@gmail.com added the comment:
Removed previously attached partial patch, this is complete patch.
Summary:
Added following 3 constants in ctypes:
ctypes.BITFIELD_ALLOCATION_NATIVE
ctypes.BITFIELD_ALLOCATION_GCC
ctypes.BITFIELD_ALLOCATION_MSVC
Setting
Changes by Vlad Riscutia riscutiav...@gmail.com:
Removed file: http://bugs.python.org/file22617/cfield_bitfield_refactoring.diff
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue12528
Vlad Riscutia riscutiav...@gmail.com added the comment:
Looks like this was implemented by design at some point. In cfield.c, we have
specific code to treat character array fields as strings:
/* Field descriptors for 'c_char * n' are be scpecial cased to
return a Python string
Vlad Riscutia riscutiav...@gmail.com added the comment:
I have a similar patch for issue 6068. I wasn't aware of this issue when I
looked into it.
I believe both patches fix the same thing (please take a look and correct me if
I'm wrong). My fix: we don't need to treat Windows differently
Vlad Riscutia riscutiav...@gmail.com added the comment:
I ran full test suit after making the _array_type = type(Array) change and
everything passes.
I also took a look at this and found additional leak. gc shows this as garbage:
[(class '_ctypes._SimpleCData',), class 'ctypes.c_longdouble
Vlad Riscutia riscutiav...@gmail.com added the comment:
Added unit test to reproduce the issue (covers both big endian and little
endian structures so _other_endian function is hit on any machine).
Test currently fails without fix and passes with proposed fix in place.
--
keywords
Vlad Riscutia riscutiav...@gmail.com added the comment:
Also added diff for fix:
- Implemented proposed issubclass(typ, Structure) solution
- Refactored _other_endian function because we used to getattr, catch
exception, then check if type is array/structure. I believe exception throwing
Changes by Vlad Riscutia riscutiav...@gmail.com:
Removed file: http://bugs.python.org/file22627/issue4376_fix.diff
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue4376
Vlad Riscutia riscutiav...@gmail.com added the comment:
New patch containing unittest that actually tests the feature.
I would appreciate it if someone can try this on a bigendian machine.
--
Added file: http://bugs.python.org/file22642/issue4376_fix.diff
Changes by Vlad Riscutia riscutiav...@gmail.com:
Removed file: http://bugs.python.org/file22642/issue4376_fix.diff
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue4376
Vlad Riscutia riscutiav...@gmail.com added the comment:
Changed c_int in tests to c_int32 just to be on the safe side.
--
Added file: http://bugs.python.org/file22643/issue4376_fix.diff
___
Python tracker rep...@bugs.python.org
http://bugs.python.org
Vlad Riscutia riscutiav...@gmail.com added the comment:
But if you set raw memory to let's say b'\0\0\0\1', when you look at the c_int
value afterwards, won't it be different on little endian and big endian
machines?
--
___
Python tracker rep
Changes by Vlad Riscutia riscutiav...@gmail.com:
Removed file: http://bugs.python.org/file22643/issue4376_fix.diff
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue4376
Vlad Riscutia riscutiav...@gmail.com added the comment:
You're right. I was busy swapping bytes in my head and missed that :)
--
Added file: http://bugs.python.org/file22644/issue4376_fix.diff
___
Python tracker rep...@bugs.python.org
http
Vlad Riscutia riscutiav...@gmail.com added the comment:
Ping? This also fixes 6493 (I believe in a cleaner way)
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue6068
Vlad Riscutia riscutiav...@gmail.com added the comment:
Updated patch to reflect review feedback. Allocation strategy is now specified
as string in Python code.
I kept asserts in CanContinueField/CanExpandField because, as I said, default
case should never be hit. Input is validated
Vlad Riscutia riscutiav...@gmail.com added the comment:
Reason you are seeing the failure for this is following change from 2.5 in
mmapmodule.c (:1109):
m_obj-data = (char *) MapViewOfFile(m_obj-map_handle,
dwDesiredAccess
Vlad Riscutia riscutiav...@gmail.com added the comment:
How come tokenizer module is not based on actual C tokenizer? Wouldn't that
make more sense (and prevent this kind of issues)?
--
nosy: +vladris
___
Python tracker rep...@bugs.python.org
http
Vlad Riscutia riscutiav...@gmail.com added the comment:
The main reason for this issue is that internally ctypes doesn't consider
c_char_p as a pointer type. This is a bit counter-intuitive, but c_char_p is
treated as a simple type and has some other sugar built-in, like easier
integration
Vlad Riscutia riscutiav...@gmail.com added the comment:
Not sure how this can be fixed actually. I don't think PyErr_SetFromErrno
instead of PyErr_SetString would work because as far as I can tell, standard
does not mention dlopen making use of errno. Unlike Windows, where LoadLibrary
Vlad Riscutia riscutiav...@gmail.com added the comment:
Attached patch for this issue.
This only happens on MSVC x64 (I actually tired to repro on Arch Linux x64
before starting work on it and it didn't repro).
What happens is that MSVC on x64 always passes structures larger than 8 bytes
Vlad Riscutia riscutiav...@gmail.com added the comment:
Changing type to behavior as it doesn't crash on 3.3. I believe issue was
opened against 2.6 and Santoso changed it to 2.7 and up where there is no crash.
Another data point: there is similar fix in current version of libffi here:
https
Vlad Riscutia riscutiav...@gmail.com added the comment:
Attached patch for 3.3 with unittest
--
keywords: +patch
nosy: +vladris
Added file: http://bugs.python.org/file22923/issue12764_patch3x.diff
___
Python tracker rep...@bugs.python.org
http
Vlad Riscutia riscutiav...@gmail.com added the comment:
Also patch for 2.7 with unittest. BTW, bx works on 2.7.
--
Added file: http://bugs.python.org/file22924/issue12764_patch2x.diff
___
Python tracker rep...@bugs.python.org
http://bugs.python.org
Vlad Riscutia riscutiav...@gmail.com added the comment:
Attached new mapping though I don't know where unit test for this should go...
--
keywords: +patch
nosy: +vladris
Added file: http://bugs.python.org/file22969/issue12802.diff
___
Python tracker
Vlad Riscutia riscutiav...@gmail.com added the comment:
Attached unit test.
--
Added file: http://bugs.python.org/file23026/issue12802_unittest.diff
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue12802
Vlad Riscutia riscutiav...@gmail.com added the comment:
I wasn't aware this is an auto-generated file. I can add a comment but looking
at it, it seems we auto-generate this file just to save a call to _dosmaperr. I
would refactor the whole function to call _dosmaperr first then if result
Vlad Riscutia riscutiav...@gmail.com added the comment:
Oh, got it. Interesting. Then should I just add a comment somewhere or should
we resolve this as Won't Fix?
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue12802
Vlad Riscutia riscutiav...@gmail.com added the comment:
Attached updated patch which extends generrmap.c to allow for easy addition of
other error mappings.
Also regenerated errmap.h and unittest.
--
Added file: http://bugs.python.org/file23054/issue12802_2.diff
Vlad Riscutia riscutiav...@gmail.com added the comment:
Ah, I see Antoine already attached a patch. I was 3 minutes late :)
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue12802
Vlad Riscutia riscutiav...@gmail.com added the comment:
Meador, I believe this was the first issue on the tracker that got me looking
into bitfield allocation. I agree that big-endian on MSVC doesn't make too much
sense but you can disregard that - using default endianess will still yield
Vlad Riscutia riscutiav...@gmail.com added the comment:
Sounds good. Please nosy me in the doc bug.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue6069
Vlad Riscutia riscutiav...@gmail.com added the comment:
Well currently we pack bitfields with an algorithm that uses #ifdefs for GCC
and MSVC builds. This feature tries to remove the hardcoded behavior and
implement it as a runtime option. This should improve interop with other
compilers
Vlad Riscutia riscutiav...@gmail.com added the comment:
I believe there is a deeper issue here in ctypes design. Basically we provide
both c_char_p and POINTER(c_char) which should behave exactly the same since
both are the equivalent of char* in C but internally they have different
Vlad Riscutia riscutiav...@gmail.com added the comment:
Attached doc update against tip (though I still hope my patch for configurable
allocation strategies will make it in).
This is my first doc patch so let me know if I missed something. I am basically
explaining that bit field allocation
Vlad Riscutia riscutiav...@gmail.com added the comment:
I attached a patch which replaces all asserts with checks that raise
exceptions. I used my judgement in determining exception types but I might have
been off in some places. Also, this patch replaces ALL asserts. It is possible
that some
Vlad Riscutia riscutiav...@gmail.com added the comment:
Thanks for the quick review! I attached second iteration addressing feedback +
changed all occurrences of checks like type(x) is y to isinstance(x, y).
I would appreciate a second look because this patch has many small changes and
even
Vlad Riscutia riscutiav...@gmail.com added the comment:
Thanks for the make patchcheck tip, I didn't know about that. I will update
the patch soon.
In the mean time, I want to point out a couple of things:
First, I'm saying toying with the underlying buffer because none of the bugs
are actual
Vlad Riscutia riscutiav...@gmail.com added the comment:
I agree compiler matters for alignment but if you look at PyCField_FromDesc,
you will see the layout is pretty much #ifdef MS_WIN32 - #else.
Sorry for generalizing, all indeed is not the right word. My point is that we
should set
47 matches
Mail list logo
