Hello,
I've addressed most of your concerns, please check the attached patch
and let me know what else can be done.
Regards,
Yan
On 7/31/23 11:44, NRK wrote:
Hi Yan,
On Sat, Jul 29, 2023 at 02:46:29PM +0200, Yan Doroshenko wrote:
I've created a patch for sxot that adds a -m (--monitor) param that allows
to select which monitor to capture in a multihead setup. Let me know what
you think.
Thanks for the patch, I don't use a multimonitor setup to test it out
properly, but there are already a couple things which aren't too good.
+ int m[1];
+ str_to_int(str_from_cstr(argv[++i]), m);
str_to_int() does certain error checking (such as overflow) and returns
false in case of failure. That return value should not be ignored. It
should fatally error out if str_to_int() returns false.
It's also weird to use `int m[1]` instead of using `int m` and then
taking a pointer.
+ XRRScreenResources *screen;
+ screen = XRRGetScreenResources (x11.dpy,
DefaultRootWindow(x11.dpy));
I'm not familiar with Xrander (and my local manpage is lacking
documentation for this function) but given that it returns a pointer, it
most likely needs to be error checked.
+ XRRCrtcInfo *crtc_info;
+ crtc_info = XRRGetCrtcInfo (x11.dpy, screen,
screen->crtcs[m[0]]);
Same here, most likely needs error checking. But even more importantly:
screen->crtcs[m[0]]
one can never assume anything about data that came from outside. There's
no guarantee that m[0] won't be bigger than the len of `screen->crtcs`.
I see that there's a `ncrtc` member, which likely contains the len of
`crtcs`. You should check to make sure that it doesn't exceed that.
If you compile with AddressSanitizer (see the "recommended debug build"
on the README) and input some absurdly high value, you'll notice the
buffer overflow:
$ ./sxot -m 1024 >/dev/null
=
==11432==ERROR: AddressSanitizer: heap-buffer-overflow on address
0x61a02600 at pc 0x00404271 bp 0x7ffe95aa74a0 sp 0x7ffe95aa7498
And even if you enter a valid value, ASan reports 2 leaks (output
cleaned up):
$ ./sxot -m 0 >/dev/null
SUMMARY: AddressSanitizer: 1296 byte(s) leaked in 2 allocation(s).
So something probably needs to be freed above.
- NRK
diff --git a/sxot.c b/sxot.c
index de87126..094cd7b 100644
--- a/sxot.c
+++ b/sxot.c
@@ -31,6 +31,7 @@
#include
#include
#include
+#include
typedef uint8_t u8;
typedef uint32_tu32;
@@ -207,7 +208,8 @@ main(int argc, char *argv[])
"usage: sxot [options]\n"
"Options:\n"
" -g, --geomCapture the specified rectangle\n"
- " -c, --curosr Capture the cursor also\n"
+ " -c, --cursor Capture the cursor also\n"
+ " -m, --monitor nCapture the specified monitor\n"
);
Str version = S(
"sxot " VERSION "\n"
@@ -237,6 +239,38 @@ main(int argc, char *argv[])
if (!parse_geom(opt.v, str_from_cstr(argv[++i]))) {
fatal(S("invalid geometry\n"));
}
+
+ } else if (str_eq(arg, S("--monitor")) || str_eq(arg, S("-m"))) {
+ int m;
+ if (!str_to_int(str_from_cstr(argv[++i]), )) {
+fatal(S("invalid monitor number\n"));
+ }
+
+ int n;
+ XRRMonitorInfo *monitors;
+ monitors = XRRGetMonitors(x11.dpy, x11.root, true, );
+
+ free(monitors);
+
+ if (n == -1) {
+fatal(S("get monitors failed\n"));
+ }
+ if (m >= n) {
+fatal(S("no monitor with such index\n"));
+ }
+
+ XRRScreenResources *screen;
+ screen = XRRGetScreenResources(x11.dpy, DefaultRootWindow(x11.dpy));
+ XRRCrtcInfo *crtc_info;
+ crtc_info = XRRGetCrtcInfo(x11.dpy, screen, screen->crtcs[m]);
+
+ opt.x = crtc_info->x;
+ opt.y = crtc_info->y;
+ opt.h = crtc_info->height;
+ opt.w = crtc_info->width;
+
+ free(crtc_info);
+ free(screen);
} else if (str_eq(arg, S("--cursor")) || str_eq(arg, S("-c"))) {
opt.capture_cursor = true;
} else if (str_eq(arg, S("--help")) || str_eq(arg, S("-h"))) {
OpenPGP_signature.asc
Description: OpenPGP digital signature
