Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > comp.lang.python > #108198 > unrolled thread

python, ctypes and GetIconInfo issue

Back to article view | Back to comp.lang.python

Contents

  python, ctypes and GetIconInfo issue mymyxin@gmail.com - 2016-05-05 13:47 -0700
    Re: python, ctypes and GetIconInfo issue eryk sun <eryksun@gmail.com> - 2016-05-05 19:09 -0500
      Re: python, ctypes and GetIconInfo issue mymyxin@gmail.com - 2016-05-06 06:36 -0700
        Re: python, ctypes and GetIconInfo issue eryk sun <eryksun@gmail.com> - 2016-05-06 18:38 -0500
      Re: python, ctypes and GetIconInfo issue mymyxin@gmail.com - 2016-05-06 07:49 -0700
        Re: python, ctypes and GetIconInfo issue eryk sun <eryksun@gmail.com> - 2016-05-06 18:39 -0500
          Re: python, ctypes and GetIconInfo issue mymyxin@gmail.com - 2016-05-07 14:58 -0700

#108198 — python, ctypes and GetIconInfo issue

Hello,

I try to make the GetIconInfo function work, but I can't figure out
what I'm doing wrong.

From the MSDN documentation the function is

https://msdn.microsoft.com/en-us/library/windows/desktop/ms648070%28v=vs.85%29.aspx

    # BOOL WINAPI GetIconInfo(
    # _In_  HICON     hIcon,
    # _Out_ PICONINFO piconinfo
    # );

which I defined as

    GetIconInfo = windll.user32.GetIconInfo
    GetIconInfo.argtypes   = [HICON, POINTER(ICONINFO)]
    GetIconInfo.restype    = BOOL
    GetIconInfo.errcheck   = ErrorIfZero


The structure piconinfo is described as
https://msdn.microsoft.com/en-us/library/windows/desktop/ms648052%28v=vs.85%29.aspx

    # typedef struct _ICONINFO {
    # BOOL    fIcon;
    # DWORD   xHotspot;
    # DWORD   yHotspot;
    # HBITMAP hbmMask;
    # HBITMAP hbmColor;
    # } ICONINFO, *PICONINFO;

my implementation is

    class ICONINFO(Structure):
        __fields__ = [
                      ('fIcon',     BOOL),
                      ('xHotspot',  DWORD),
                      ('yHotspot',  DWORD),
                      ('hbmMask',   HBITMAP),
                      ('hbmColor',  HBITMAP),
                     ]
                     
#### not part of the problem but needed to get the icon handle    
    hicon = ImageList_GetIcon(def_il_handle,1,ILD_NORMAL)
    print hicon
####

As the documentation states, the function run successful if return code is
none zero. Well I get 1 returned but as soon as I try to access a class member
the program crashes.
    
    iconinfo = ICONINFO()
    lres =  GetIconInfo(hicon, pointer(iconinfo))
    print lres
    print '{0}'.format(sizeof(iconinfo))   # <- crash

If I comment the print of sizeof... the program keeps running but if I call
the same code a second time then it crashes at GetIconInfo(hicon, ...)

So it looks like I'm doing something terribly wrong but don't see it.

Can someone shed some light on it?

Thank you
Hubert

[toc] | [next] | [standalone]

#108203

On Thu, May 5, 2016 at 3:47 PM,  <mymyxin@gmail.com> wrote:
>
> I try to make the GetIconInfo function work, but I can't figure out
> what I'm doing wrong.
>
> From the MSDN documentation the function is
>
> https://msdn.microsoft.com/en-us/library/windows/desktop/ms648070%28v=vs.85%29.aspx
>
>     # BOOL WINAPI GetIconInfo(
>     # _In_  HICON     hIcon,
>     # _Out_ PICONINFO piconinfo
>     # );
>
> which I defined as
>
>     GetIconInfo = windll.user32.GetIconInfo
>     GetIconInfo.argtypes   = [HICON, POINTER(ICONINFO)]
>     GetIconInfo.restype    = BOOL
>     GetIconInfo.errcheck   = ErrorIfZero

Please avoid windll. It caches the loaded library, which in turn
caches function pointers. So all packages that use windll.user32 are
potentially stepping on each others' toes with mutually incompatible
function prototypes. It also doesn't allow configuring
use_last_error=True to enable ctypes.get_last_error() for WinAPI
function calls.

> The structure piconinfo is described as
> https://msdn.microsoft.com/en-us/library/windows/desktop/ms648052%28v=vs.85%29.aspx
>
>     # typedef struct _ICONINFO {
>     # BOOL    fIcon;
>     # DWORD   xHotspot;
>     # DWORD   yHotspot;
>     # HBITMAP hbmMask;
>     # HBITMAP hbmColor;
>     # } ICONINFO, *PICONINFO;
>
> my implementation is
>
>     class ICONINFO(Structure):
>         __fields__ = [
>                       ('fIcon',     BOOL),
>                       ('xHotspot',  DWORD),
>                       ('yHotspot',  DWORD),
>                       ('hbmMask',   HBITMAP),
>                       ('hbmColor',  HBITMAP),
>                      ]

The attribute name is "_fields_", not "__fields__", so you haven't
actually defined any fields and sizeof(ICONINFO) is 0. When you pass
this empty struct to GetIconInfo, it potentially overwrites and
corrupts existing data on the heap that can lead to a crash later on.

Here's the setup I created to test GetIconInfo and GetIconInfoEx.
Maybe you can reuse some of this code, but if you're using XP this
won't work as written because GetIconInfoEx was added in Vista.

Note the use of a __del__ finalizer to call DeleteObject on the
bitmaps. Otherwise, in a real application, calling GetIconInfo would
leak memory. Using __del__ is convenient, but note that you can't
reuse an instance without manually calling DeleteObject on the
bitmaps.

    import ctypes
    from ctypes import wintypes

    kernel32 = ctypes.WinDLL('kernel32', use_last_error=True)
    user32 = ctypes.WinDLL('user32', use_last_error=True)
    gdi32 = ctypes.WinDLL('gdi32')

    MAX_PATH = 260
    IMAGE_ICON = 1

    class ICONINFO_BASE(ctypes.Structure):
        def __del__(self, gdi32=gdi32):
            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_BASE):
        _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))

        def __init__(self, *args, **kwds):
            super(ICONINFOEX, self).__init__(*args, **kwds)
            self.cbSize = ctypes.sizeof(self)

    PICONINFO = ctypes.POINTER(ICONINFO)
    PICONINFOEX = ctypes.POINTER(ICONINFOEX)

    def check_bool(result, func, args):
        if not result:
            raise ctypes.WinError(ctypes.get_last_error())
        return args

    kernel32.GetModuleHandleW.errcheck = check_bool
    kernel32.GetModuleHandleW.restype = wintypes.HMODULE
    kernel32.GetModuleHandleW.argtypes = (
        wintypes.LPCWSTR,) # _In_opt_ lpModuleName

    # DeleteObject doesn't call SetLastError
    gdi32.DeleteObject.restype = wintypes.BOOL
    gdi32.DeleteObject.argtypes = (
       wintypes.HGDIOBJ,) # _In_ hObject

    user32.LoadImageW.errcheck = check_bool
    user32.LoadImageW.restype = wintypes.HANDLE
    user32.LoadImageW.argtypes = (
        wintypes.HINSTANCE, # _In_opt_ hinst
        wintypes.LPCWSTR,   # _In_     lpszName
        wintypes.UINT,      # _In_     uType
        ctypes.c_int,       # _In_     cxDesired
        ctypes.c_int,       # _In_     cyDesired
        wintypes.UINT,)     # _In_     fuLoad

    user32.DestroyIcon.errcheck = check_bool
    user32.DestroyIcon.restype = wintypes.BOOL
    user32.DestroyIcon.argtypes = (
        wintypes.HICON,) # _In_ hIcon

    user32.GetIconInfo.errcheck = check_bool
    user32.GetIconInfo.restype = wintypes.BOOL
    user32.GetIconInfo.argtypes = (
        wintypes.HICON, # _In_  hIcon
        PICONINFO,)     # _Out_ piconinfo

    # requires Vista+
    user32.GetIconInfoExW.errcheck = check_bool
    user32.GetIconInfoExW.restype = wintypes.BOOL
    user32.GetIconInfoExW.argtypes = (
        wintypes.HICON, # _In_  hIcon
        PICONINFOEX,)   # _Out_ piconinfoex

    if __name__ == '__main__':
        hMain = kernel32.GetModuleHandleW(None)
        hIcon = user32.LoadImageW(hMain, wintypes.LPCWSTR(1),
                                  IMAGE_ICON, 0, 0, 0)
        try:
            info = ICONINFOEX()
            user32.GetIconInfoExW(hIcon, ctypes.byref(info))
            print('fIcon    : %d' % info.fIcon)
            print('wResID   : %d' % info.wResID)
            print('szModName: %s' % info.szModName)
        finally:
            user32.DestroyIcon(hIcon)


The __main__ test outputs the following for me in 3.5 and 2.7:

    fIcon    : 1
    wResID   : 1
    szModName: C:\Program Files\Python35\python.exe

    fIcon    : 1
    wResID   : 1
    szModName: C:\Program Files\Python27\python.exe

[toc] | [prev] | [next] | [standalone]

#108224

Hello eryk sun,
first of all thank you very much, really appreciate your help.
Please be informed that I'm a python beginner, so forgive me if
my following questions sound stupid (also I'm not a native speaker).

> Please avoid windll. It caches the loaded library, which in turn 
> caches function pointers. So all packages that use windll.user32 are 
> potentially stepping on each others' toes with mutually incompatible 
> function prototypes. It also doesn't allow configuring 
> use_last_error=True to enable ctypes.get_last_error() for WinAPI 
> function calls. 
I assume you are referring to this block of code

    GetIconInfo = windll.user32.GetIconInfo
    GetIconInfo.argtypes   = [HICON, POINTER(ICONINFO)]
    GetIconInfo.restype    = BOOL
    GetIconInfo.errcheck   = ErrorIfZero

where as you use

    user32 = ctypes.WinDLL('user32', use_last_error=True) 
    user32.GetIconInfoExW.errcheck = check_bool 
    user32.GetIconInfoExW.restype = wintypes.BOOL 
    user32.GetIconInfoExW.argtypes = ( 
        wintypes.HICON, # _In_  hIcon 
        PICONINFOEX,)   # _Out_ piconinfoex 
        
I've checked ctype docu included in python but don't find any hint about your concerns.
May I ask you, do you know additional documents/sites which I can use to get a better understanding
about caching issue? Or did I miss something from used documentation?

> The attribute name is "_fields_", not "__fields__", so you haven't 
> actually defined any fields and sizeof(ICONINFO) is 0. When you pass 
> this empty struct to GetIconInfo, it potentially overwrites and 
> corrupts existing data on the heap that can lead to a crash later on. 

Ahhh, typically me. Thank you for pointing to it.

> Here's the setup I created to test GetIconInfo and GetIconInfoEx. 
> Maybe you can reuse some of this code, but if you're using XP this 
> won't work as written because GetIconInfoEx was added in Vista. 

> Note the use of a __del__ finalizer to call DeleteObject on the 
> bitmaps. Otherwise, in a real application, calling GetIconInfo would 
> leak memory. 

Oh, you answered already an upcoming question I guess, thank you ;-)

> Using __del__ is convenient, but note that you can't 
> reuse an instance without manually calling DeleteObject on the 
> bitmaps. 
Don't understand this. Isn't this covered by your example in base class?

    class ICONINFO_BASE(ctypes.Structure): 
        def __del__(self, gdi32=gdi32): 
            if self.hbmMask: 
                gdi32.DeleteObject(self.hbmMask) 
                self.hbmMask = None 
            if self.hbmColor: 
                gdi32.DeleteObject(self.hbmColor) 
                self.hbmColor = None 
                
If I would do somthing like

    iconinfoex = ICONINFOEX()
    iconinfoex = None
    
DeleteObject would be called once None gets assigned, wouldn't it?

Again, thank you very much.
Hubert

[toc] | [prev] | [next] | [standalone]

#108245

On Fri, May 6, 2016 at 8:36 AM,  <mymyxin@gmail.com> wrote:
>
>> Please avoid windll. It caches the loaded library, which in turn
>> caches function pointers. So all packages that use windll.user32 are
>> potentially stepping on each others' toes with mutually incompatible
>> function prototypes. It also doesn't allow configuring
>> use_last_error=True to enable ctypes.get_last_error() for WinAPI
>> function calls.
> I assume you are referring to this block of code
>
>     GetIconInfo = windll.user32.GetIconInfo
>     GetIconInfo.argtypes   = [HICON, POINTER(ICONINFO)]
>     GetIconInfo.restype    = BOOL
>     GetIconInfo.errcheck   = ErrorIfZero
>
> where as you use
>
>     user32 = ctypes.WinDLL('user32', use_last_error=True)
>     user32.GetIconInfoExW.errcheck = check_bool
>     user32.GetIconInfoExW.restype = wintypes.BOOL
>     user32.GetIconInfoExW.argtypes = (
>         wintypes.HICON, # _In_  hIcon
>         PICONINFOEX,)   # _Out_ piconinfoex
>
> I've checked ctype docu included in python but don't find any hint about your concerns.
> May I ask you, do you know additional documents/sites which I can use to get a better
> understanding about caching issue? Or did I miss something from used documentation?

You haven't missed anything in the documentation. The ctypes docs need
work, and some of the examples are bad, if not wrong. For example, the
GetModuleHandleA examples incorrectly handle the pointer result
because they were never updated for 64-bit Windows. One can't use a
Python function as the restype with a C function that returns a
pointer because it will be truncated to a C int. Whoever wrote the
GetModuleHandleA examples either doesn't know how this feature is
implemented in ctypes (probably not, since I think Thomas Heller wrote
the example), or doesn't know that a Windows HMODULE is a pointer to
the module's base address, or was just writing sloppy code in the era
of 32-bit Windows.

In this case, look at the CDLL [1] and LibraryLoader [2] classes. Note
how CDLL.__getattr__ caches function pointers using setattr(self,
name, func). Note how LibraryLoader.__getattr__  caches libraries
using setattr(self, name, dll), and how it instantiates the library
using self._dlltype(name), with no way to specify use_last_error=True.
This has caused real problems for projects such as colorama (fixed)
and pyreadline (still broken), and I've seen potential problems in
several other projects that naively copy the cdll and windll examples
from the docs. It's not their fault. The docs are just bad on this
subject.

[1]: https://hg.python.org/cpython/file/v3.5.1/Lib/ctypes/__init__.py#l314
[2]: https://hg.python.org/cpython/file/v3.5.1/Lib/ctypes/__init__.py#l410

>> Using __del__ is convenient, but note that you can't
>> reuse an instance without manually calling DeleteObject on the
>> bitmaps.
>
> Don't understand this. Isn't this covered by your example in base class?

I'm talking about reusing an instance, to avoid the cost of repeated
allocation and deallocation. For example:

    info = ICONINFOEX()
    for hIcon in hIcons:
        user32.GetIconInfoExW(hIcon, ctypes.byref(info))
        print('fIcon    : %d' % info.fIcon)
        print('wResID   : %d' % info.wResID)
        print('szModName: %s' % info.szModName)
        gdi32.DeleteObject(info.hbmMask)
        gdi32.DeleteObject(info.hbmColor)

[toc] | [prev] | [next] | [standalone]

#108230

A further question if you don't mind.

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)) 

    def __init__(self, *args, **kwds): 
        super(ICONINFOEX, self).__init__(*args, **kwds) 
        self.cbSize = ctypes.sizeof(self)

A dir on instance of that class indicates that it should be possible
as it contains all needed members and size seems to be correct as well,
but a call to GetIconInfoExW fails with:

WindowsError: [Error 87] The parameter is incorrect. 


        info = ICONINFOEX()
        info.cbSize = ctypes.sizeof(info) 
        print dir(info)
        print info.cbSize
        user32.GetIconInfoExW(hIcon, ctypes.byref(info)) 

Thank you
Hubert

[toc] | [prev] | [next] | [standalone]

#108246

On Fri, May 6, 2016 at 9:49 AM,  <mymyxin@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

[toc] | [prev] | [next] | [standalone]

#108303

Hello eryk sun,

thank you very much for your help and detailed answers.
With the provided links and useful information I should be
able to get a better understanding how ctypes works internally.

Thank you
Hubert

[toc] | [prev] | [standalone]

Back to top | Article view | comp.lang.python

csiph-web