[issue12880] ctypes: clearly document how structure bit fields are allocated
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 issues of the form I created this bitfield structure with Python, passed it to C function but C structure was different. That would be a bitfield bug. All of these bugs are people setting raw memory to some bytes, then looking at bitfield members and not seeing what they expect. Since this is platform dependent, they shouldn't worry about the raw memory as long as C interop works fine. Bitfield layout is complex as it involves both allocation algorithm and structure packing and same Python code will work differently on Windows and Unix. My point is that documenting all this low-level stuff will encourage people to work with the raw memory which will open the door for other issues. I believe it would be better to encourage users to stick to declaring members and accessing them by name as raw memory WILL be different for the same code on different OSes. Second, one of your review comments is: GCC is used for most Unix systems and Microsoft VC++ is used on Windows.. This is not how ctypes works. Ctypes implements the bitfield allocation algorithm itself, it doesn't use the compiler with which it is built. Basically it says #ifdef WIN32 - allocate like VC++ - #else - allocate like GCC. So it doesn't really matter with which compiler you are building Python. It will still do GCC style allocation on Solaris. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue12880 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue12880] ctypes: clearly document how structure bit fields are allocated
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 expectation correctly - VC++-style on Windows, GCC-style everywhere else and encourage users to access structure members by name, not raw memory. Issues opened for bitfields *usually* are of the form I mentioned - setting raw memory to some bytes then seeing members are not what user expected, even if ctypes algorithm works correctly. As I said, I will revise the patch and maybe make it more clear that users should look up how bitfield allocation works for their compiler instead of trying to understand this via structure raw memory. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue12880 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5001] Remove assertion-based checking in multiprocessing
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 of the internal functions should be kept using asserts but I am not familiar enough with the module to say which. I figured I'd better do the whole thing than reviewers can say where lines should be reverted to asserts. -- keywords: +patch nosy: +vladris Added file: http://bugs.python.org/file23298/issue5001.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5001 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5001] Remove assertion-based checking in multiprocessing
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 though I ran full test suit which passed, I'm afraid I made a typo somewhere :) -- Added file: http://bugs.python.org/file23302/issue5001_v2.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5001 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue12880] ctypes: clearly document how structure bit fields are allocated
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 is compiler-specific and no assumptions should be made of how a bitfield is allocated. I believe this is the better thing to do rather than detailing how GCC and MSVC allocated their bitfields because that would just encourage people to use this feature incorrectly. Most bugs opened on bit fields are because people are toying with the underlying buffer and get other results than what they expect. IMO, when using bitfields one should only access the structure members at a high level and not go read/write the raw memory underneath. -- keywords: +patch Added file: http://bugs.python.org/file23275/bitfield_doc.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue12880 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5149] syntactic sugar: type coercion on pointer assignment
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 implementations. c_char_p is considered a simple type and I believe supports some conversions to and from Python strings while POINTER(c_char) is considered a pointer type which supports assignment from array etc. I think a better fix would be to deprecate p_char_p or make it an equivalent of POINTER(c_char), otherwise we will have to do work on c_char_p to make it more like POINTER(c_char) when issues like this get opened and probably also make POINTER(c_char) more like c_char_p. Why not just have POINTER(c_char) which works as expected? I don't have all the historical context on why this pseudo-simple type was provided but I saw a couple of issues where people expect it to behave like a C char* but it won't because it is implemented as a convenience type with limited support. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5149 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue6069] casting error from ctypes array to structure
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 different sizes of bitfields when compiled with GCC and MSVC. Basically bitfield allocation is compiler specific and patch in issue12528 implements a way to select which allocation strategy to be used at runtime instead of hardcoding the one with which Python is compiled. This should improve cross-compiler interop. I wanted to hyperlink that patch to all other bitfield bugs, that's why I followed up with link to the patch. Feel free to close this, either as not an issue or as a duplicate of issue12528. And yes, this bit about bitfield allocation should be documented and I was planning to look into it at some point after 12528 gets committed. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue6069 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue6069] casting error from ctypes array to structure
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 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue12528] Implement configurable bitfield allocation strategy
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. Currently I provided these for MSVC-style and GCC-style bitfield allocations. These, of course, can be extended with other strategies. I am not sure that the fact that GCC has different types of bitfield allocations in different versions is a point against this feature. Consider that in our current code we don't use compiler bitfield allocation, we create the structure layout using our own algorithm, interop might be broken even if Python gets built with same version of GCC as the binary we want to interop with as long as algorithm is out of date. This patch should provide some flexibility in this matter. Wouldn't a GCC44 constant provided at API level be better than saying you can't interop with anything build with GCC 4.4 and up? Or vice-versa, anything built with GCC 4.4. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue12528 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue12802] Windows error code 267 should be mapped to ENOTDIR, not EINVAL
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 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue12802] Windows error code 267 should be mapped to ENOTDIR, not EINVAL
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 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue12802 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue12802] Windows error code 267 should be mapped to ENOTDIR, not EINVAL
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 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue12802] Windows error code 267 should be mapped to ENOTDIR, not EINVAL
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 is
still EINVAL, tweak with custom switch case. The way I see it, this looks like
premature optimization since OS error shouldn't be on a hot code path, meaning
an application should be able to live with an extra CRT function call on such
exceptions. I'm willing to implement this if there are no objections.
Something like:
errno = _dosmaperr(err)
if (EINVAL == errno)
{
switch (err)
{
// Our tweaks
}
}
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue12802
___
___
Python-bugs-list mailing list
Unsubscribe:
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue12802] Windows error code 267 should be mapped to ENOTDIR, not EINVAL
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 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue12802] Windows error code 267 should be mapped to ENOTDIR, not EINVAL
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 rep...@bugs.python.org http://bugs.python.org/issue12802 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue12764] segfault in ctypes.Struct with bad _fields_
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://bugs.python.org/issue12764 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue12764] segfault in ctypes.Struct with bad _fields_
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/issue12764 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11835] python (x64) ctypes incorrectly pass structures parameter
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
by reference. See here:
http://msdn.microsoft.com/en-us/library/ms235286(v=vs.90).aspx
Now this was accounted for in callproc.c, line 1143 in development branch with
this:
if (atypes[i]-type == FFI_TYPE_STRUCT
#ifdef _WIN64
atypes[i]-size = sizeof(void *)
#endif
)
avalues[i] = (void *)args[i].value.p;
else
avalues[i] = (void *)args[i].value;
This fix wasn't made in libffi_msvc/ffi.c though. Here, regardless of whether
we have x64 or x86 build, if z = sizeof(int) we will hit else branch in
libffi_msvc/ffi.c at line 114 and do:
else
{
memcpy(argp, *p_argv, z);
}
p_argv++;
argp += z;
In our case, we copy 28 bytes as arguments (size of our structure) but in fact
for x64 we only need 8 as structure is passed by reference so argument is just
a pointer. My patch will adjust z before hitting if statement on x64 and it
will cause correct copy as pointer.
--
nosy: +vladris
Added file: http://bugs.python.org/file22899/issue11835_patch.diff
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue11835
___
___
Python-bugs-list mailing list
Unsubscribe:
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11835] python (x64) ctypes incorrectly pass structures parameter
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://github.com/atgreen/libffi/blob/master/.pc/win64-struct-args/src/x86/ffi.c Since at the moment we are not integrating new libffi, I believe my fix should do (libffi fix is slightly different but I'm matching what we have in callproc.c which is not part of libffi). -- type: crash - behavior ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11835 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5149] syntactic sugar: type coercion on pointer assignment
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 with Python strings. Alternative pointer type is POINTER(c_char), which allows assignment from array. I would make this clear in the documentation. Even now, documentation says following about c_char_p: Represents the C char * datatype when it points to a zero-terminated string. For a general character pointer that may also point to binary data, POINTER(c_char) must be used. The constructor accepts an integer address, or a string. So there already is a hint that c_char_p has limited use for convenience. We should maybe make documentation more clear. If you want equivalent of C char* behavior, POINTER(c_char) is the way to go. -- nosy: +vladris ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5149 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue12437] _ctypes.dlopen does not include errno in OSError
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 will change last-error, if dlopen fails, only way to retrieve error is to dlerror which returns a string, no error code. -- nosy: +vladris ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue12437 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue12675] tokenize module happily tokenizes code with syntax errors
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://bugs.python.org/issue12675 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue12562] calling mmap twice fails on Windows
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, 0, 0, 0); changed to mmapmodule.c (:1414 in 3.3): m_obj-data = (char *) MapViewOfFile(m_obj-map_handle, dwDesiredAccess, off_hi, off_lo, m_obj-size); Previously size wasn't passed to MapViewOfFile. Passing new size to MapViewOfFile greater than the size of previous map causes an error. This seems to be by design. From MSDN: MapViewOfFile: dwNumberOfBytesToMap [in] The number of bytes of a file mapping to map to the view. All bytes must be within the maximum size specified by CreateFileMapping. If this parameter is 0 (zero), the mapping extends from the specified offset to the end of the file mapping. CreateFileMapping: lpName [in, optional] The name of the file mapping object. If this parameter matches the name of an existing mapping object, the function requests access to the object with the protection that flProtect specifies. So on second call, CreateFileMapping will get back the previous mapping object, which has 4096 bytes of memory mapped. MapViewOfFile will try to map beyond its limit and get an error. I am curious how resizing worked before. I tried passing size=0 to MapViewOfFile on second call (length=8192) then call VirtualQuery on the returned map, which can query the size of the buffer. Size is still 4096. So even if length=8192 and we call CreateFileMapping with this length, it will return the previous 4096 byte-buffer. This looks to me like an issue which existed until Python 2.5, namely this error was silenced and returned map was still 4096 bytes, just claiming to be 8192. The way it is behaving now seems to be the correct way. -- nosy: +vladris ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue12562 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue12528] Implement configurable bitfield allocation strategy
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 in stgdict and this should make it clear to whoever reads the code that something is very wrong if execution gets to default case. -- Added file: http://bugs.python.org/file22728/configurable_bitfield_allocation_v2.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue12528 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue6068] ctypes is not correctly handling bitfields backed by 64 bit integers on Windows
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 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue4376] Nested ctypes 'BigEndianStructure' fails
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 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue4376] Nested ctypes 'BigEndianStructure' fails
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 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue4376 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue4376] Nested ctypes 'BigEndianStructure' fails
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 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue4376] Nested ctypes 'BigEndianStructure' fails
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/issue4376 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue4376] Nested ctypes 'BigEndianStructure' fails
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...@bugs.python.org http://bugs.python.org/issue4376 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue4376] Nested ctypes 'BigEndianStructure' fails
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 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue4376] Nested ctypes 'BigEndianStructure' fails
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://bugs.python.org/issue4376 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue4376] Nested ctypes 'BigEndianStructure' fails
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: +patch nosy: +vladris Added file: http://bugs.python.org/file22623/issue4376_test.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue4376 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue4376] Nested ctypes 'BigEndianStructure' fails
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 should not be on normal code path so I replaced try-except with a check for hasattr - Removed a unittest which becomes deprecated with this fix (unittest asserts getting _other_endian for nested struct raises exception which is no longer the case) -- Added file: http://bugs.python.org/file22627/issue4376_fix.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue4376 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue12528] Implement configurable bitfield allocation strategy
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 not defined by
standard. MSVC and GCC have different strategies to perform the allocation so
the size of bitfield structures can be different depending on compiler.
Currently we hardcode allocation strategy to be GCC-way on non-Windows and
MSVC-way on Windows which raises issues when trying to interop on Windows with
GCC binaries.
Short term this solution will enable interop between MSVC compiled Python with
GCC compiled binaries under Windows. It will also enable addressing other
possible compiler interop issues in the future, for compilers that don't use
GCC strategy.
Following is copied from thread discussing this:
On 6/25/2011 12:33 PM, Vlad Riscutia wrote:
I recently started looking at some ctypes issues. I dug a bit into
http://bugs.python.org/issue6069 and then I found
http://bugs.python.org/issue11920. They both boil down to the fact that
bitfield allocation is up to the compiler, which is different in GCC and
MSVC. Currently we have hard-coded allocation strategy based on paltform
in cfields.c:
if (bitsize /* this is a bitfield request */
*pfield_size /* we have a bitfield open */
#ifdef MS_WIN32
/* MSVC, GCC with -mms-bitfields */
dict-size * 8 == *pfield_size
#else
/* GCC */
dict-size * 8= *pfield_size
#endif
(*pbitofs + bitsize)= *pfield_size) {
/* continue bit field */
fieldtype = CONT_BITFIELD;
#ifndef MS_WIN32
} else if (bitsize /* this is a bitfield request */
*pfield_size /* we have a bitfield open */
dict-size * 8= *pfield_size
(*pbitofs + bitsize)= dict-size * 8) {
/* expand bit field */
fieldtype = EXPAND_BITFIELD;
#endif
So when creating a bitfield structure, it's size can be different on
Linux vs Windows.
class MyStructure(ctypes.BigEndianStructure):
_pack_ = 1# aligned to 8 bits, not ctypes default of 32
_fields_= [
(Data0, ctypes.c_uint32, 32),
(Data1, ctypes.c_uint8, 3),
(Data2, ctypes.c_uint16, 12),
]
sizeof for above structure is 6 on GCC build and 7 on MSVC build. This
leads to some confusion and issues, because we can't always interop
correctly with code compiled with different compiler than the one Python
is compiled with on the platform.
Just curious, are you saying that this is the 'cause' of the two bug reports,
or 'just' something you discovered while investigating them?
Short term solution is to add a warning in the documentation about this.
For 2.7/3.2, yes.
Longer term though, I think it
would be better to add a property on the Structure class for
configurable allocation strategy, for example Native (default), GCC,
MSVC and when allocating the bitfield, use given strategy. Native would
behave similar to what happens now, but we would also allow GCC-style
allocation on Windows for example and the ability to extend this if we
ever run into similar issues with other compilers. This shouldn't be too
difficult to implement, will be backwards compatible and it would
improve interop. I would like to hear some opinions on this.
If this would allow the MSVC-compilied Python to better access dlls compiled
with gcc (cygwin) on Windows, definitely -- in 3.3.
If the new feature is (currently) only useful on Windows, doc should say so.
--
Terry Jan Reedy
/copy
Attached is patch with initial refactoring of cfield.c to enable configurable
allocation. Next step is to provide a way to configure this through Python
library. I will also look at updating documentation to point out the known
issue.
--
components: ctypes
files: cfield_bitfield_refactoring.diff
keywords: patch
messages: 140088
nosy: terry.reedy, vladris
priority: normal
severity: normal
status: open
title: Implement configurable bitfield allocation strategy
type: feature request
versions: Python 3.3
Added file: http://bugs.python.org/file22617/cfield_bitfield_refactoring.diff
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue12528
___
___
Python-bugs-list mailing list
Unsubscribe:
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11920] ctypes: Strange bitfield structure sizing issue
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 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue6069] casting error from ctypes array to structure
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 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue12528] Implement configurable bitfield allocation strategy
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 _bitfield_allocation_ attribute to one of these on a class declaration inheriting from Structure will force specified allocation of the bitfield. NATIVE is equivalent to not specifying anything. GCC will do GCC-style allocation (what Python does now on non-Windows) MSVC will do MSVC-style allocation (what Python does now on Windows) I added unittests to cover these and ran full suit on both Windows and Linux. Still have to update documentation to mention this. Will submit diff for that after this gets reviewed. -- Added file: http://bugs.python.org/file22618/configurable_bitfield_allocation.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue12528 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue12528] Implement configurable bitfield allocation strategy
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 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue8161] inconsistency behavior in ctypes.c_char_p dereferencing
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 instead of an Array object instance...
*/
if (PyCArrayTypeObject_Check(proto)) {
StgDictObject *adict = PyType_stgdict(proto);
StgDictObject *idict;
if (adict adict-proto) {
idict = PyType_stgdict(adict-proto);
if (!idict) {
PyErr_SetString(PyExc_TypeError,
has no _stginfo_);
Py_DECREF(self);
return NULL;
}
if (idict-getfunc == _ctypes_get_fielddesc(c)-getfunc) {
struct fielddesc *fd = _ctypes_get_fielddesc(s);
getfunc = fd-getfunc;
setfunc = fd-setfunc;
}
#ifdef CTYPES_UNICODE
if (idict-getfunc == _ctypes_get_fielddesc(u)-getfunc) {
struct fielddesc *fd = _ctypes_get_fielddesc(U);
getfunc = fd-getfunc;
setfunc = fd-setfunc;
}
#endif
}
}
Simple fix would be to just remove this whole section though this might break
code which relied on string assignment to such fields. For example:
class T(ctypes.Structure):
_fields_ = (
('member', ctypes.c_char * 16),
)
x = T()
x.member = bytes('Spam', 'ascii')
Above works now but will fail if change is made. There is a high chance this
would break existing code as I imagine people using this due to convenience. An
alternative would be to keep string setfunc but don't change getfunc, though
that is also pretty inconsistent as you will be able to set a string but not
get one back.
If we are willing to take the risk, fix is easy.
--
nosy: +vladris
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue8161
___
___
Python-bugs-list mailing list
Unsubscribe:
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue6493] Can not set value for structure members larger than 32 bits
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, just remove #ifdef and #define BIT_MASK(size) ((1LL NUM_BITS(size))-1) regardless of platform. Unittests for this patch pass for my patch too. I believe this is some old #ifdef that was put in place due to a compiler bug which got fixed since then. -- nosy: +vladris ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue6493 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue12142] Reference cycle when importing ctypes
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', attribute '_
_dict__' of 'c_longdouble' objects, attribute '__weakref__' of 'c_longdouble'
objects, (class 'ctypes.c_longdouble', class '_ctypes._SimpleCData', class
'_ctypes._CData', class 'object'), {'__dict__': attribute '__dict__' of 'c_
longdouble' objects, '_type_': 'g', '__module__': 'ctypes', '__weakref__': att
ribute '__weakref__' of 'c_longdouble' objects, '__doc__': None}]
This is all caused by these lines in ctypes __init__.py:
class c_longdouble(_SimpleCData):
_type_ = g
if sizeof(c_longdouble) == sizeof(c_double):
c_longdouble = c_double
For me sizeof(c_longdouble) == sizeof(c_double) (I believe MS compiler always
does this) but when we assign c_longdouble = c_double, there is a leak. I
removed the alias lines:
if sizeof(c_longdouble) == sizeof(c_double):
c_longdouble = c_double
And the leak was gone. Looks like changing c_longdouble after declaring it
causes a leak. Below for similar aliasing of longlong types, we have this:
if _calcsize(l) == _calcsize(q):
# if long and long long have the same size, make c_longlong an alias for
c_long
c_longlong = c_long
c_ulonglong = c_ulong
else:
class c_longlong(_SimpleCData):
_type_ = q
_check_size(c_longlong)
class c_ulonglong(_SimpleCData):
_type_ = Q
This avoids declaring c_longlong and c_ulonglong as class if not needed to. The
problem is _calcsize(g) causes an error because g is used as long double
througout ctypes but _calcsize is function from _struct.c, where g (long
double) is not defined. Not sure why it isn't...
So in short:
As far as I can tell _array_type = type(Array) doesn't break anything
Looks like we have another leak in ctypes (which isn't a big deal)
We have elegant fix for the leak once _struct.c will support long double
--
nosy: +vladris
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue12142
___
___
Python-bugs-list mailing list
Unsubscribe:
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue6068] ctypes is not correctly handling bitfields backed by 64 bit integers on Windows
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_ = [(a, c_ulonglong, 16), (b, c_ulonglong, 32), (c, c_ulonglong, 16)] s = X() s.b = 1 print(s.b) # this prints 0 Whenever field width goes over 32 bits, setting or getting value of the field in cfield.c will only look at last (actual size - 32) bits of the field. So if we have a field of 34 bits, only least significant 2 bits will be operated on. The above repro results in an (actual size - 32) = 0 bits so nothing is getting set with the assignement. This is not caught in unit test package because we have only this in test_bitfields.py: def test_ulonglong(self): class X(Structure): _fields_ = [(a, c_ulonglong, 1), (b, c_ulonglong, 62), (c, c_ulonglong, 1)] self.assertEqual(sizeof(X), sizeof(c_longlong)) x = X() self.assertEqual((x.a, x.b, x.c), (0, 0, 0)) x.a, x.b, x.c = 7, 7, 7 self.assertEqual((x.a, x.b, x.c), (1, 7, 1)) For 62 bit width, we will actually operate on last 30 bits but this test passes as 7 fits in those bits. If we would actually try to set it to 0x3FFF, result will be different than expected (0x3FFF). I will look into extending UT package with some tests that set full range of bits and check if they are actually being set correctly. This issue reproes with latest bits but only on Windows. Root cause seems to be BIT_MASK macro in cfield.c which is ifdef-ed to something different on Windows. I patched on my machine by removing the special treatment: @@ -429,12 +429,7 @@ #define LOW_BIT(x) ((x) 0x) #define NUM_BITS(x) ((x) 16) -/* This seems nore a compiler issue than a Windows/non-Windows one */ -#ifdef MS_WIN32 -# define BIT_MASK(size) ((1 NUM_BITS(size))-1) -#else -# define BIT_MASK(size) ((1LL NUM_BITS(size))-1) -#endif +#define BIT_MASK(size) ((1LL NUM_BITS(size))-1) /* This macro CHANGES the first parameter IN PLACE. For proper sign handling, we must first shift left, then right. Unittests still pass with this in place and now fields are handled correctly though I don't know why this was put in in the first place. Looking at file history it seems it was there from the start (from 2006). Could it be that it was addressing some MSVC bug which got fixed in the meantime? Things seem to be building and working fine now when using 1LL for shift. Also related to this I have doubts about the SWAP_8 macro which is similarly changed for MSVC, also in cfield.c. I am only able to build 32 bit version so I can't say whether this was put in place due to some x64 issue, maybe someone can check behavior on x64 build. If that's the case, maybe #ifdef should take that into account. -- nosy: +vladris title: support read/write c_ulonglong type bitfield structures - ctypes is not correctly handling bitfields backed by 64 bit integers on Windows type: feature request - behavior versions: +Python 3.3 -Python 3.2 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue6068 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue6068] ctypes is not correctly handling bitfields backed by 64 bit integers on Windows
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 and was asking for a feature to make them read-write. That is not the case, behavior is caused by a bug. By UT I mean unittest. I reproed this on 64bit Windows 7 on latest sources with x32 build (as I said, I don't have x64 build). -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue6068 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue6068] ctypes is not correctly handling bitfields backed by 64 bit integers on Windows
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 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue6068 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue6069] casting error from ctypes array to structure
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 of 32
_fields_= [
(Data0, ctypes.c_uint32, 32),
(Data1, ctypes.c_uint8, 3),
(Data2, ctypes.c_uint16, 12),
]
But you also have this assumption when generating the test data:
size_of_structures_in_bytes = 6
I verified and this does not hold with MSVC compiler. Using VC++ 2005, this code
typedef struct Test {
unsigned int x: 32; // uint_32 : 32
unsigned char y: 3; // uint_8 : 3
unsigned short int z: 12; // uint_16 : 12
} Test;
gives sizeof(Test) == 7. In Python, if you look at sizeof(closest_fit), it will
also be 7.
Looking at cfield.c, seems this was taken into account when creating bit fields:
if (bitsize /* this is a bitfield request */
*pfield_size /* we have a bitfield open */
#ifdef MS_WIN32
/* MSVC, GCC with -mms-bitfields */
dict-size * 8 == *pfield_size
#else
/* GCC */
dict-size * 8 = *pfield_size
#endif
(*pbitofs + bitsize) = *pfield_size) {
/* continue bit field */
fieldtype = CONT_BITFIELD;
#ifndef MS_WIN32
} else if (bitsize /* this is a bitfield request */
*pfield_size /* we have a bitfield open */
dict-size * 8 = *pfield_size
(*pbitofs + bitsize) = dict-size * 8) {
/* expand bit field */
fieldtype = EXPAND_BITFIELD;
#endif
} else if (bitsize) {
/* start new bitfield */
fieldtype = NEW_BITFIELD;
*pbitofs = 0;
*pfield_size = dict-size * 8;
Though I don't know this first-hand, above code plus sizeof experiment leads me
to believe that gcc packs bitfields differently than MSVC. Seems that gcc will
expand existing bitfield trying to pack structure more tightly so indeed on
Linux (or I assume Windows gcc build), size of this structure is 6 as gcc will
combine these seeing that an unsigned short can hold all 15 bits required but
with MSVC this won't work. MSVC will allocate both the c_uint8 and the c_uint16
once is sees that last 12 bits don't fit in remaining c_uint8.
As far as I can tell this is by design and Python matches expected MSVC
structure packing for this test case.
--
nosy: +vladris
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue6069
___
___
Python-bugs-list mailing list
Unsubscribe:
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue6068] ctypes is not correctly handling bitfields backed by 64 bit integers on Windows
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 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue6068 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
