python, ctypes and GetIconInfo issue

[eryk sun]

On Fri, May 6, 2016 at 9:49 AM,  <mymyxin at gmail.com> wrote:
>
> In your example you used a base class
> and ICONINFO well as ICONINFOEX inherit it.
> As the members of ICONINFO are part of ICONINFOEX
> couldn't we do something like
>
> class ICONINFO_BASE(ctypes.Structure):
>     def __del__(self, ):
>         if self.hbmMask:
>             gdi32.DeleteObject(self.hbmMask)
>             self.hbmMask = None
>         if self.hbmColor:
>             gdi32.DeleteObject(self.hbmColor)
>             self.hbmColor = None
>
> class ICONINFO(ICONINFO_BASE):
>     _fields_ = (('fIcon',    wintypes.BOOL),
>                 ('xHotspot', wintypes.DWORD),
>                 ('yHotspot', wintypes.DWORD),
>                 ('hbmMask',  wintypes.HBITMAP),
>                 ('hbmColor', wintypes.HBITMAP))
>
> class ICONINFOEX(ICONINFO):
>     _fields_ = (('cbSize',    wintypes.DWORD),
>                 # ('fIcon',     wintypes.BOOL),
>                 # ('xHotspot',  wintypes.DWORD),
>                 # ('yHotspot',  wintypes.DWORD),
>                 # ('hbmMask',   wintypes.HBITMAP),
>                 # ('hbmColor',  wintypes.HBITMAP),
>                 ('wResID',    wintypes.WORD),
>                 ('szModName', wintypes.WCHAR * MAX_PATH),
>                 ('szResName', wintypes.WCHAR * MAX_PATH))

In this case, cbSize field will be offset after hbmColor:

    >>> ICONINFOEX.hbmColor.offset
    24
    >>> ICONINFOEX.cbSize.offset
    32

A struct subclass appends its fields to the base class fields. In
theory, you can do this in some cases, but in practice I don't
recommend it (see below).

For example, look at SHARE_INFO_0 [1], SHARE_INFO_1 [2], and
SHARE_INFO_2 [3], which are used to query different levels of
information about network shares.

[1]: https://msdn.microsoft.com/en-us/library/bb525402
[2]: https://msdn.microsoft.com/en-us/library/bb525407
[3]: https://msdn.microsoft.com/en-us/library/bb525408

It can help to maintain a consistent type hierarchy, such as in the
following answer that I wrote to list network shares on Windows:

http://stackoverflow.com/a/36848031/205580

When ctypes checks the type of a pointer argument, it first checks
whether its _type_ is a subclass of the _type_ of the corresponding
pointer type in argtypes. If not, it falls back on checking whether
the pointer argument itself is an instance of the argtypes pointer
type. Similarly, for a byref() argument it checks whether the referent
is an instance of the _type_ of the argtypes pointer type. Maintaining
a consistent type hierarchy provides type safety without having to
tediously cast pointers.

However, I don't recommend subclassing to append _fields_ because it
has a bug. The code that updates the StgDictObject (i.e. the subclass
of dict used by ctypes types for FFI storgage info) for structs and
union types doesn't doesn't properly initialize the ffi_type elements
array. The length field of the stgdict needs to be the total number of
fields, inclusive of all base classes, in order to copy the entire
ffi_type elements array from the base class. However, storing the
total length in the stgdict's length field would require rewriting the
way an instance of a struct is recursively initialized over the base
classes.

This bug affects passing structs by value. This isn't common (passing
unions by value isn't even supported), so I haven't bothered to submit
a patch for this. Here's an example crash on 64-bit Linux:

test.c:

    #include <stdio.h>

    typedef struct _data_t {
        int x, y, z;
    } data_t;

    int test(data_t d) {
        printf("%d, %d, %d\n", d.x, d.y, d.z);
        return 0;
    }

test.py:

    from ctypes import *

    lib = CDLL('./test.so')

    class A(Structure):
        _fields_ = (('a', c_int), ('b', c_int), ('c', c_int),)

    class B(Structure):
        _fields_ = (('a', c_int),)
    class C(B):
        _fields_ = (('b', c_int),)
    class D(C):
        _fields_ = (('c', c_int),)

    print('test A')
    lib.test(A(42, 84, 168))

    print('test D')
    lib.test(D(42, 84, 168))

output:
    test A
    42, 84, 168
    test D
    Aborted