Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MSS._is_extension_enabled() suspicious try/except #245

Closed
mgorny opened this issue Apr 15, 2023 · 2 comments
Closed

MSS._is_extension_enabled() suspicious try/except #245

mgorny opened this issue Apr 15, 2023 · 2 comments

Comments

@mgorny
Copy link
Contributor

mgorny commented Apr 15, 2023

General information:

  • OS name: Gentoo Linux
  • OS version: n/a
  • OS architecture: amd64
  • Resolutions: n/a
  • Python version: n/a
  • MSS version: git (ef97a27)

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
Copy link
Owner

BoboTiG commented Apr 15, 2023

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
Copy link
Contributor Author

mgorny commented Apr 15, 2023

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

@BoboTiG BoboTiG closed this as completed Apr 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants