BoboTiG / python-mss

An ultra fast cross-platform multiple screenshots module in pure Python using ctypes.
https://pypi.org/project/mss/
MIT License
987 stars 88 forks source link

MSS._is_extension_enabled() suspicious try/except #245

Closed mgorny closed 1 year ago

mgorny commented 1 year ago

General information:

Description of the warning/error

While debugging a segfault (I'll file a separate bug about it later, unless it's a PEBKAC), I've noticed the following suspicious code:

class MSS(MSSBase):
    # ...
    def _is_extension_enabled(self, name: str, /) -> bool:
        """Return True if the given *extension* is enabled on the server."""
        with lock:
            major_opcode_return = c_int()
            first_event_return = c_int()
            first_error_return = c_int()

            try:
                self.xlib.XQueryExtension(
                    self._handles.display,
                    name.encode("latin1"),
                    byref(major_opcode_return),
                    byref(first_event_return),
                    byref(first_error_return),
                )
            except ScreenShotError:
                return False
            return True

(https://github.com/BoboTiG/python-mss/blob/v8.0.3/src/mss/linux.py#L358-L367)

I'm sorry if I'm missing something non-obvious but it looks that this is calling the libX11 function directly via CDLL, and as such it can't raise ScreenShotError but it rather returns a boolean result.

BoboTiG commented 1 year ago

Hello @mgorny,

Good question!

Actually, when setting up C functions, we define errcheck to check all calls: https://github.com/BoboTiG/python-mss/blob/v8.0.3/src/mss/linux.py#L381 The callback for errcheck will then raise a ScreenShotError if the C function failed: https://github.com/BoboTiG/python-mss/blob/v8.0.3/src/mss/linux.py#L233

At the end, the code you are pointing is legit. It's not obvious at first sight :)

mgorny commented 1 year ago

Ah, ok then. I finally was able to figure the segv some more, so I'll file a bug in a minute ;-).