From e2f5a7b53296df8d93134ee77d35697f92e1e532 Mon Sep 17 00:00:00 2001 From: Cendio Date: Wed, 20 Dec 2017 15:58:49 +0100 Subject: [PATCH 1/8] Refactor geometry string parsing Signed-off-by: Henrik Andersson Signed-off-by: Karl Mikaelsson --- rdesktop.c | 197 ++++++++++++++++++++++--------- tests/Makefile | 7 +- tests/cache_mock.c | 6 + tests/cliprdr_mock.c | 8 ++ tests/ctrl_mock.c | 18 +++ tests/disk_mock.c | 8 ++ tests/lspci_mock.c | 9 ++ tests/parallel_mock.c | 8 ++ tests/parse_geometry_test.c | 228 ++++++++++++++++++++++++++++++++++++ tests/printer_mock.c | 8 ++ tests/rdp_mock.c | 24 ++++ tests/rdpdr_mock.c | 6 + tests/rdpedisp_mock.c | 6 + tests/secure_mock.c | 12 ++ tests/serial_mock.c | 8 ++ tests/tcp_mock.c | 6 + tests/ui_mock.c | 48 ++++++++ tests/utils_mock.c | 19 +++ tests/xkeymap_mock.c | 6 + xwin.c | 11 +- 20 files changed, 576 insertions(+), 67 deletions(-) create mode 100644 tests/cliprdr_mock.c create mode 100644 tests/disk_mock.c create mode 100644 tests/lspci_mock.c create mode 100644 tests/parallel_mock.c create mode 100644 tests/parse_geometry_test.c create mode 100644 tests/printer_mock.c create mode 100644 tests/serial_mock.c create mode 100644 tests/utils_mock.c diff --git a/rdesktop.c b/rdesktop.c index 8ea0f6b..d992f43 100644 --- a/rdesktop.c +++ b/rdesktop.c @@ -592,6 +592,144 @@ parse_server_and_port(char *server) } +// [WxH|P%|W%xH%][@DPI][+X[+Y]]|workarea +int parse_geometry_string(const char *optarg) +{ + sint32 value; + const char *ps; + char *pe; + + /* special keywords */ + if (strcmp(optarg, "workarea") == 0) + { + g_sizeopt = 1; + return 0; + } + + /* parse first integer */ + ps = optarg; + value = strtol(ps, &pe, 10); + if (ps == pe || value <= 0) + { + logger(Core, Error, "invalid geometry, expected positive integer for width"); + return -1; + } + + g_initial_width = value; + ps = pe; + + /* expect % or x */ + if (*ps != '%' && *ps != 'x') + { + logger(Core, Error, "invalid geometry, expected '%%' or 'x' after width"); + return -1; + } + + if (*ps == '%') + { + g_sizeopt = -1; + ps++; + pe++; + } + + if (*ps == 'x') + { + ps++; + value = strtol(ps, &pe, 10); + if (ps == pe || value <= 0) + { + logger(Core, Error, "invalid geometry, expected positive integer for height"); + return -1; + } + + g_initial_height = value; + ps = pe; + + if (*ps == '%' && g_sizeopt == 0) + { + logger(Core, Error, "invalid geometry, unexpected '%%' after height"); + return -1; + } + + if (g_sizeopt == -1) + { + if (*ps != '%') + { + logger(Core, Error, "invalid geometry, expected '%%' after height"); + return -1; + } + ps++; + pe++; + } + } + else + { + if (g_sizeopt == -1) + { + /* same percentage of screen for both width and height */ + g_initial_height = g_initial_width; + } + else + { + logger(Core, Error, "invalid geometry, missing height (WxH)"); + return -1; + } + } + + /* parse optional dpi */ + if (*ps == '@') + { + ps++; + pe++; + value = strtol(ps, &pe, 10); + if (ps == pe || value <= 0) + { + logger(Core, Error, "invalid geometry, expected positive integer for DPI"); + return -1; + } + + g_dpi = value; + ps = pe; + } + + /* parse optional window position */ + if (*ps == '+' || *ps == '-') + { + /* parse x position */ + value = strtol(ps, &pe, 10); + if (ps == pe) + { + logger(Core, Error, "invalid geometry, expected an integer for X position"); + return -1; + } + + g_pos |= (value < 0) ? 2 : 1; + g_xpos = value; + ps = pe; + } + + if (*ps == '+' || *ps == '-') + { + /* parse y position */ + value = strtol(ps, &pe, 10); + if (ps == pe) + { + logger(Core, Error, "invalid geometry, expected an integer for Y position"); + return -1; + } + g_pos |= (value < 0) ? 4 : 1; + g_ypos = value; + ps = pe; + } + + if (*pe != '\0') + { + logger(Core, Error, "invalid geometry, unexpected characters at end of string"); + return -1; + } + return 0; +} + /* Client program */ int main(int argc, char *argv[]) @@ -709,67 +847,10 @@ main(int argc, char *argv[]) case 'g': geometry_option = True; g_fullscreen = False; - if (!strcmp(optarg, "workarea")) + if (parse_geometry_string(optarg) != 0) { - g_sizeopt = 1; - break; - } - - g_initial_width = strtol(optarg, &p, 10); - if (g_initial_width <= 0) - { - logger(Core, Error, "invalid geometry width specified"); return EX_USAGE; } - - if (*p == 'x') - g_initial_height = strtol(p + 1, &p, 10); - - if (g_initial_height <= 0) - { - logger(Core, Error, "invalid geometry height specified"); - return EX_USAGE; - } - - if (*p == '%') - { - g_sizeopt = -g_initial_width; - g_initial_width = g_sizeopt; - - if (*(p + 1) == 'x') - { - g_initial_height = -strtol(p + 2, &p, 10); - } - else - { - g_initial_height = g_sizeopt; - } - - p++; - } - - if (*p == '@') - { - g_dpi = strtol(p + 1, &p, 10); - if (g_dpi <= 0) - { - logger(Core, Error, "invalid DPI: expected a positive integer after @\n"); - return EX_USAGE; - } - } - - if (*p == '+' || *p == '-') - { - g_pos |= (*p == '-') ? 2 : 1; - g_xpos = strtol(p, &p, 10); - - } - if (*p == '+' || *p == '-') - { - g_pos |= (*p == '-') ? 4 : 1; - g_ypos = strtol(p, NULL, 10); - } - break; case 'f': diff --git a/tests/Makefile b/tests/Makefile index 8e52b47..d131181 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -2,7 +2,7 @@ CC=gcc CFLAGS=-fPIC -Wall -Wextra -ggdb -gdwarf-2 -g3 CGREEN_RUNNER=cgreen-runner -TESTS=resize rdp xwin utils +TESTS=resize rdp xwin utils parse_geometry RDP_MOCKS=ui_mock.o bitmap_mock.o secure_mock.o ssl_mock.o mppc_mock.o \ @@ -19,6 +19,9 @@ RESIZE_MOCKS=x11_mock.o cache_mock.o xclip_mock.o xkeymap_mock.o seamless_mock.o ssl_mock.o mppc_mock.o pstcache_mock.o orders_mock.o rdesktop_mock.o rdp5_mock.o \ tcp_mock.o licence_mock.o mcs_mock.o channels_mock.o +PARSE_MOCKS=ui_mock.o rdpdr_mock.o rdpedisp_mock.o ssl_mock.o ctrl_mock.o secure_mock.o \ + tcp_mock.o dvc_mock.o rdp_mock.o cache_mock.o cliprdr_mock.o disk_mock.o lspci_mock.o \ + parallel_mock.o printer_mock.o serial_mock.o xkeymap_mock.o utils_mock.o all: test @@ -43,6 +46,8 @@ utils: utils_test.o $(UTILS_MOCKS) resize: resize_test.o $(RESIZE_MOCKS) $(CC) $(CFLAGS) -shared -lcgreen -o $@ $^ -lX11 -lXcursor +parse_geometry: parse_geometry_test.o $(PARSE_MOCKS) ../rdesktop.c + $(CC) $(CFLAGS) -shared -lcgreen -o $@ parse_geometry_test.o $(PARSE_MOCKS) .PHONY: clean clean: diff --git a/tests/cache_mock.c b/tests/cache_mock.c index ba28194..dc46fdd 100644 --- a/tests/cache_mock.c +++ b/tests/cache_mock.c @@ -44,3 +44,9 @@ cache_put_desktop(uint32 offset, int cx, int cy, int scanline, { mock(offset, cx, cy, scanline, bytes_per_pixel, data); } + +void +cache_save_state() +{ + mock(); +} diff --git a/tests/cliprdr_mock.c b/tests/cliprdr_mock.c new file mode 100644 index 0000000..001f010 --- /dev/null +++ b/tests/cliprdr_mock.c @@ -0,0 +1,8 @@ +#include +#include "../rdesktop.h" + +void +cliprdr_set_mode(const char *optarg) +{ + mock(optarg); +} diff --git a/tests/ctrl_mock.c b/tests/ctrl_mock.c index 89e7684..8e326fc 100644 --- a/tests/ctrl_mock.c +++ b/tests/ctrl_mock.c @@ -12,3 +12,21 @@ ctrl_check_fds(fd_set * rfds, fd_set * wfds) { mock(rfds, wfds); } + +int +ctrl_init(const char *user, const char *domain, const char *host) +{ + return mock(user, domain, host); +} + +RD_BOOL +ctrl_is_slave() +{ + return mock(); +} + +int +ctrl_send_command(const char *cmd, const char *args) +{ + return mock(cmd, args); +} diff --git a/tests/disk_mock.c b/tests/disk_mock.c new file mode 100644 index 0000000..1a01120 --- /dev/null +++ b/tests/disk_mock.c @@ -0,0 +1,8 @@ +#include +#include "../rdesktop.h" + +int +disk_enum_devices(uint32 *id, char *optarg) +{ + return mock(id, optarg); +} diff --git a/tests/lspci_mock.c b/tests/lspci_mock.c new file mode 100644 index 0000000..19141fe --- /dev/null +++ b/tests/lspci_mock.c @@ -0,0 +1,9 @@ +#include +#include "../rdesktop.h" + + +RD_BOOL +lspci_init(void) +{ + return mock(); +} diff --git a/tests/parallel_mock.c b/tests/parallel_mock.c new file mode 100644 index 0000000..1072a05 --- /dev/null +++ b/tests/parallel_mock.c @@ -0,0 +1,8 @@ +#include +#include "../rdesktop.h" + +int +parallel_enum_devices(uint32 * id, char *optarg) +{ + return mock(id, optarg); +} diff --git a/tests/parse_geometry_test.c b/tests/parse_geometry_test.c new file mode 100644 index 0000000..90fe93f --- /dev/null +++ b/tests/parse_geometry_test.c @@ -0,0 +1,228 @@ +#include +#include +#include "../rdesktop.h" +#include "../proto.h" +#include +#include + +#define always_expect_error_log() always_expect(logger, when(lvl, is_equal_to(Error))) + +/* Boilerplate */ +Describe(ParseGeometry); +BeforeEach(ParseGeometry) {}; +AfterEach(ParseGeometry) {}; + +/* Global Variables.. :( */ +int g_tcp_port_rdp; +RDPDR_DEVICE g_rdpdr_device[16]; +uint32 g_num_devices; +char *g_rdpdr_clientname; +RD_BOOL g_using_full_workarea; + +#define PACKAGE_VERSION "test" + +#include "../rdesktop.c" + + +Ensure(ParseGeometry, HandlesWxH) +{ + g_initial_width = g_initial_height = g_sizeopt = 0; + + assert_that(parse_geometry_string("1234x2345"), is_equal_to(0)); + + assert_that(g_initial_width, is_equal_to(1234)); + assert_that(g_initial_height, is_equal_to(2345)); + assert_that(g_sizeopt, is_equal_to(0)); +} + +Ensure(ParseGeometry, FailsOnMissingHeight) +{ + always_expect_error_log(); + + g_initial_width = g_initial_height = g_sizeopt = 0; + assert_that(parse_geometry_string("1234"), is_equal_to(-1)); + + assert_that(g_initial_width, is_equal_to(1234)); + assert_that(g_initial_height, is_equal_to(0)); + assert_that(g_sizeopt, is_equal_to(0)); +} + +Ensure(ParseGeometry, FailsOnMissingHeightVariant2) +{ + always_expect_error_log(); + + g_initial_width = g_initial_height = g_sizeopt = 0; + assert_that(parse_geometry_string("1234x"), is_equal_to(-1)); + + assert_that(g_initial_width, is_equal_to(1234)); + assert_that(g_initial_height, is_equal_to(0)); + assert_that(g_sizeopt, is_equal_to(0)); +} + +Ensure(ParseGeometry, HandlesPercentageOfScreen) +{ + g_initial_width = g_initial_height = g_sizeopt = 0; + + assert_that(parse_geometry_string("80%"), is_equal_to(0)); + + assert_that(g_initial_width, is_equal_to(80)); + assert_that(g_initial_height, is_equal_to(80)); + assert_that(g_sizeopt, is_equal_to(-1)); +} + +Ensure(ParseGeometry, HandlesSpecificWidthAndHeightPercentageOfScreen) +{ + g_initial_width = g_initial_height = g_sizeopt = 0; + + assert_that(parse_geometry_string("100%x60%"), is_equal_to(0)); + + assert_that(g_initial_width, is_equal_to(100)); + assert_that(g_initial_height, is_equal_to(60)); + assert_that(g_sizeopt, is_equal_to(-1)); +} + +Ensure(ParseGeometry, HandlesSpecifiedDPI) +{ + g_dpi = g_initial_width = g_initial_height = g_sizeopt = 0; + + assert_that(parse_geometry_string("1234x2345@234"), is_equal_to(0)); + + assert_that(g_dpi, is_equal_to(234)); + assert_that(g_initial_width, is_equal_to(1234)); + assert_that(g_initial_height, is_equal_to(2345)); + assert_that(g_sizeopt, is_equal_to(0)); +} + + +Ensure(ParseGeometry, HandlesSpecifiedXPosition) +{ + g_xpos = g_ypos = g_initial_width = g_initial_height = g_sizeopt = 0; + + assert_that(parse_geometry_string("1234x2345+123"), is_equal_to(0)); + + assert_that(g_xpos, is_equal_to(123)); + assert_that(g_ypos, is_equal_to(0)); + assert_that(g_pos, is_equal_to(1)); + assert_that(g_initial_width, is_equal_to(1234)); + assert_that(g_initial_height, is_equal_to(2345)); + assert_that(g_sizeopt, is_equal_to(0)); +} + +Ensure(ParseGeometry, HandlesSpecifiedNegativeXPosition) +{ + g_ypos = g_xpos = g_initial_width = g_initial_height = g_sizeopt = 0; + + assert_that(parse_geometry_string("1234x2345-500"), is_equal_to(0)); + + assert_that(g_xpos, is_equal_to(-500)); + assert_that(g_ypos, is_equal_to(0)); + assert_that(g_pos, is_equal_to(2)); + assert_that(g_initial_width, is_equal_to(1234)); + assert_that(g_initial_height, is_equal_to(2345)); + assert_that(g_sizeopt, is_equal_to(0)); +} + +Ensure(ParseGeometry, HandlesSpecifiedNegativeXAndYPosition) +{ + g_ypos = g_xpos = g_initial_width = g_initial_height = g_sizeopt = 0; + + assert_that(parse_geometry_string("1234x2345-500-501"), is_equal_to(0)); + + assert_that(g_xpos, is_equal_to(-500)); + assert_that(g_ypos, is_equal_to(-501)); + assert_that(g_pos, is_equal_to(2 | 4)); + assert_that(g_initial_width, is_equal_to(1234)); + assert_that(g_initial_height, is_equal_to(2345)); + assert_that(g_sizeopt, is_equal_to(0)); +} + +Ensure(ParseGeometry, HandlesSpecifiedXandYPosition) +{ + g_xpos = g_ypos = g_initial_width = g_initial_height = g_sizeopt = 0; + + assert_that(parse_geometry_string("1234x2345+123+234"), is_equal_to(0)); + + assert_that(g_xpos, is_equal_to(123)); + assert_that(g_ypos, is_equal_to(234)); + assert_that(g_pos, is_equal_to(1)); + assert_that(g_initial_width, is_equal_to(1234)); + assert_that(g_initial_height, is_equal_to(2345)); + assert_that(g_sizeopt, is_equal_to(0)); +} + +Ensure(ParseGeometry, HandlesSpecifiedXandYPositionWithDPI) +{ + g_dpi = g_xpos = g_ypos = g_initial_width = g_initial_height = g_sizeopt = 0; + + assert_that(parse_geometry_string("1234x2345@678+123+234"), is_equal_to(0)); + + assert_that(g_dpi, is_equal_to(678)); + assert_that(g_xpos, is_equal_to(123)); + assert_that(g_ypos, is_equal_to(234)); + assert_that(g_initial_width, is_equal_to(1234)); + assert_that(g_initial_height, is_equal_to(2345)); + assert_that(g_sizeopt, is_equal_to(0)); +} + +Ensure(ParseGeometry, HandlesSpecialNameWorkarea) +{ + assert_that(parse_geometry_string("workarea"), is_equal_to(0)); + + assert_that(g_sizeopt, is_equal_to(1)); +} + + +Ensure(ParseGeometry, FailsOnNegativeDPI) +{ + always_expect_error_log(); + + assert_that(parse_geometry_string("1234x2345@-105"), is_equal_to(-1)); +} + + +Ensure(ParseGeometry, FailsOnNegativeWidth) +{ + always_expect_error_log(); + + assert_that(parse_geometry_string("-1234x2345"), is_equal_to(-1)); +} + + +Ensure(ParseGeometry, FailsOnNegativeHeight) +{ + always_expect_error_log(); + + assert_that(parse_geometry_string("1234x-2345"), is_equal_to(-1)); +} + +Ensure(ParseGeometry, FailsOnMixingPixelsAndPercents) +{ + always_expect_error_log(); + + g_sizeopt = 0; + assert_that(parse_geometry_string("1234%x2345"), is_equal_to(-1)); + + g_sizeopt = 0; + assert_that(parse_geometry_string("1234x2345%"), is_equal_to(-1)); +} + +Ensure(ParseGeometry, FailsOnGarbageAtEndOfString) +{ + always_expect_error_log(); + + g_sizeopt = 0; + assert_that(parse_geometry_string("1234%1239123081232345abcdefgadkfjafa4af048"), is_equal_to(-1)); + + g_sizeopt = 0; + assert_that(parse_geometry_string("1235abcer9823461"), is_equal_to(-1)); + + g_sizeopt = 0; + assert_that(parse_geometry_string("1235%x123%+123123+123123asdkjfasdf"), is_equal_to(-1)); + + g_sizeopt = 0; + assert_that(parse_geometry_string("1235%x123%@123asdkjfasdf"), is_equal_to(-1)); + + g_sizeopt = 0; + assert_that(parse_geometry_string("1235%x123%@123+1-2asdkjfasdf"), is_equal_to(-1)); +} + diff --git a/tests/printer_mock.c b/tests/printer_mock.c new file mode 100644 index 0000000..f059536 --- /dev/null +++ b/tests/printer_mock.c @@ -0,0 +1,8 @@ +#include +#include "../rdesktop.h" + +int +printer_enum_devices(uint32 * id, char *optarg) +{ + return mock(id, optarg); +} diff --git a/tests/rdp_mock.c b/tests/rdp_mock.c index 45b0eec..14a5ef6 100644 --- a/tests/rdp_mock.c +++ b/tests/rdp_mock.c @@ -13,3 +13,27 @@ rdp_send_suppress_output_pdu(enum RDP_SUPPRESS_STATUS allowupdates) mock(allowupdates); } +RD_BOOL +rdp_connect(char *server, uint32 flags, char *domain, char *password, char *command, + char *directory, RD_BOOL reconnect) +{ + return mock(server, flags, domain, password, command, directory, reconnect); +} + +void +rdp_disconnect() +{ + mock(); +} + +void +rdp_main_loop(RD_BOOL * deactivated, uint32 * ext_disc_reason) +{ + mock(deactivated, ext_disc_reason); +} + +void +rdp_reset_state() +{ + mock(); +} diff --git a/tests/rdpdr_mock.c b/tests/rdpdr_mock.c index b73c743..43b1c45 100644 --- a/tests/rdpdr_mock.c +++ b/tests/rdpdr_mock.c @@ -12,3 +12,9 @@ rdpdr_check_fds(fd_set * rfds, fd_set * wfds, RD_BOOL timed_out) { mock(rfds, wfds, timed_out); } + +RD_BOOL +rdpdr_init() +{ + return mock(); +} diff --git a/tests/rdpedisp_mock.c b/tests/rdpedisp_mock.c index 6e52377..4033f24 100644 --- a/tests/rdpedisp_mock.c +++ b/tests/rdpedisp_mock.c @@ -1,6 +1,12 @@ #include #include "../rdesktop.h" +void +rdpedisp_init() +{ + mock(); +} + RD_BOOL rdpedisp_is_available() { diff --git a/tests/secure_mock.c b/tests/secure_mock.c index b219b16..8d19c7d 100644 --- a/tests/secure_mock.c +++ b/tests/secure_mock.c @@ -30,3 +30,15 @@ void sec_send(STREAM s, uint32 flags) { mock(s, flags); } + +void +sec_hash_sha1_16(uint8 * out, uint8 * in, uint8 * salt1) +{ + mock(out, in, salt1); +} + +void +sec_hash_to_string(char *out, int out_size, uint8 * in, int in_size) +{ + mock(out, out_size, in, in_size); +} diff --git a/tests/serial_mock.c b/tests/serial_mock.c new file mode 100644 index 0000000..9a07ab8 --- /dev/null +++ b/tests/serial_mock.c @@ -0,0 +1,8 @@ +#include +#include "../rdesktop.h" + +int +serial_enum_devices(uint32 * id, char *optarg) +{ + return mock(id, optarg); +} diff --git a/tests/tcp_mock.c b/tests/tcp_mock.c index 49a101c..f35740b 100644 --- a/tests/tcp_mock.c +++ b/tests/tcp_mock.c @@ -5,3 +5,9 @@ char *tcp_get_address() { return (char *) mock(); } + +void +tcp_run_ui(RD_BOOL run) +{ + mock(run); +} diff --git a/tests/ui_mock.c b/tests/ui_mock.c index bb53320..0cb74b8 100644 --- a/tests/ui_mock.c +++ b/tests/ui_mock.c @@ -70,3 +70,51 @@ void ui_set_clip(int x,int y, int cx, int cy) { mock(x,y,cx,cy); } + +RD_BOOL +ui_create_window(uint32 width, uint32 height) +{ + return mock(width, height); +} + +void +ui_deinit() +{ + mock(); +} + +void +ui_destroy_window() +{ + mock(); +} + +void +ui_init_connection() +{ + mock(); +} + +RD_BOOL +ui_have_window() +{ + return mock(); +} + +RD_BOOL +ui_init() +{ + return mock(); +} + +void +ui_reset_clip() +{ + mock(); +} + +void +ui_seamless_end() +{ + mock(); +} diff --git a/tests/utils_mock.c b/tests/utils_mock.c new file mode 100644 index 0000000..d79102e --- /dev/null +++ b/tests/utils_mock.c @@ -0,0 +1,19 @@ +#include +#include + +#include "../rdesktop.h" + +uint32 utils_djb2_hash(const char *str) { return mock(str); } +char *utils_string_escape(const char *str) { return (char *)mock(str); } +char *utils_string_unescape(const char *str) { return (char *)mock(str); } +int utils_locale_to_utf8(const char *src, size_t is, char *dest, size_t os) { return mock(src, is, dest, os); } +int utils_mkdir_safe(const char *path, int mask) { return mock(path, mask); } +int utils_mkdir_p(const char *path, int mask) { return mock(path, mask); } +void utils_calculate_dpi_scale_factors(uint32 width, uint32 height, uint32 dpi, + uint32 *physwidth, uint32 *physheight, + uint32 *desktopscale, uint32 *devicescale) { mock(width, height, dpi, physwidth, physheight, desktopscale, devicescale); } +void utils_apply_session_size_limitations(uint32 *width, uint32 *height) { mock(width, height); } + +void logger(log_subject_t c, log_level_t lvl, char *format, ...) { mock(c, lvl, format); } +void logger_set_verbose(int verbose) { mock(verbose); } +void logger_set_subjects(char *subjects) { mock(subjects); } diff --git a/tests/xkeymap_mock.c b/tests/xkeymap_mock.c index e815721..f0db87e 100644 --- a/tests/xkeymap_mock.c +++ b/tests/xkeymap_mock.c @@ -61,3 +61,9 @@ uint16 ui_get_numlock_state(unsigned int state) { return mock(state); } + +RD_BOOL +xkeymap_from_locale(const char *locale) +{ + return mock(locale); +} diff --git a/xwin.c b/xwin.c index 474c774..8602c34 100644 --- a/xwin.c +++ b/xwin.c @@ -1983,14 +1983,9 @@ ui_init_connection(void) if (-g_sizeopt >= 100) g_using_full_workarea = True; - if (g_initial_width > 0) - g_initial_width = g_sizeopt; - - if (g_initial_height > 0) - g_initial_height = g_sizeopt; - - g_initial_height = HeightOfScreen(g_screen) * (-g_initial_height) / 100; - g_initial_width = WidthOfScreen(g_screen) * (-g_initial_width) / 100; + /* g_initial_width/height holds percentage of screen in each axis */ + g_initial_height = HeightOfScreen(g_screen) * g_initial_height / 100; + g_initial_width = WidthOfScreen(g_screen) * g_initial_width / 100; } else if (g_sizeopt == 1) { From ba41f749c6f97ea3e9d350a5f63cd6b4fa49798c Mon Sep 17 00:00:00 2001 From: Cendio Date: Thu, 21 Dec 2017 14:36:18 +0100 Subject: [PATCH 2/8] Refactor g_sizeopt into an enumeration for clarity Signed-off-by: Henrik Andersson Signed-off-by: Karl Mikaelsson --- rdesktop.c | 22 ++++++------- tests/parse_geometry_test.c | 61 +++++++++++++++++++------------------ tests/resize_test.c | 2 +- tests/xwin_test.c | 2 +- types.h | 7 +++++ xwin.c | 19 +++++------- 6 files changed, 59 insertions(+), 54 deletions(-) diff --git a/rdesktop.c b/rdesktop.c index d992f43..f6c647c 100644 --- a/rdesktop.c +++ b/rdesktop.c @@ -67,10 +67,6 @@ unsigned int g_keylayout = 0x409; /* Defaults to US keyboard layout */ int g_keyboard_type = 0x4; /* Defaults to US keyboard layout */ int g_keyboard_subtype = 0x0; /* Defaults to US keyboard layout */ int g_keyboard_functionkeys = 0xc; /* Defaults to US keyboard layout */ -int g_sizeopt = 0; /* If non-zero, a special size has been - requested. If 1, the geometry will be fetched - from _NET_WORKAREA. If negative, absolute value - specifies the percent of the whole screen. */ int g_dpi = 0; /* device DPI: default not set */ /* Following variables holds the initial width and height for a @@ -79,6 +75,9 @@ int g_dpi = 0; /* device DPI: default not set */ uint32 g_initial_width = 1024; uint32 g_initial_height = 768; +window_size_type_t g_window_size_type = Fixed; + + int g_xpos = 0; int g_ypos = 0; int g_pos = 0; /* 0 position unspecified, @@ -602,7 +601,7 @@ int parse_geometry_string(const char *optarg) /* special keywords */ if (strcmp(optarg, "workarea") == 0) { - g_sizeopt = 1; + g_window_size_type = Workarea; return 0; } @@ -627,7 +626,7 @@ int parse_geometry_string(const char *optarg) if (*ps == '%') { - g_sizeopt = -1; + g_window_size_type = PercentageOfScreen; ps++; pe++; } @@ -645,13 +644,13 @@ int parse_geometry_string(const char *optarg) g_initial_height = value; ps = pe; - if (*ps == '%' && g_sizeopt == 0) + if (*ps == '%' && g_window_size_type == Fixed) { logger(Core, Error, "invalid geometry, unexpected '%%' after height"); return -1; } - if (g_sizeopt == -1) + if (g_window_size_type == PercentageOfScreen) { if (*ps != '%') { @@ -664,9 +663,9 @@ int parse_geometry_string(const char *optarg) } else { - if (g_sizeopt == -1) + if (g_window_size_type == PercentageOfScreen) { - /* same percentage of screen for both width and height */ + /* percentage of screen used for both width and height */ g_initial_height = g_initial_width; } else @@ -1178,7 +1177,8 @@ main(int argc, char *argv[]) logger(Core, Error, "You cannot use -4 and -A at the same time"); return EX_USAGE; } - g_sizeopt = -100; + + g_window_size_type = Fullscreen; g_grab_keyboard = False; } diff --git a/tests/parse_geometry_test.c b/tests/parse_geometry_test.c index 90fe93f..bf94569 100644 --- a/tests/parse_geometry_test.c +++ b/tests/parse_geometry_test.c @@ -26,77 +26,78 @@ RD_BOOL g_using_full_workarea; Ensure(ParseGeometry, HandlesWxH) { - g_initial_width = g_initial_height = g_sizeopt = 0; + g_initial_width = g_initial_height = 0; assert_that(parse_geometry_string("1234x2345"), is_equal_to(0)); assert_that(g_initial_width, is_equal_to(1234)); assert_that(g_initial_height, is_equal_to(2345)); - assert_that(g_sizeopt, is_equal_to(0)); + assert_that(g_window_size_type, is_equal_to(Fixed)); } Ensure(ParseGeometry, FailsOnMissingHeight) { + always_expect_error_log(); - g_initial_width = g_initial_height = g_sizeopt = 0; + g_initial_width = g_initial_height = 0; assert_that(parse_geometry_string("1234"), is_equal_to(-1)); assert_that(g_initial_width, is_equal_to(1234)); assert_that(g_initial_height, is_equal_to(0)); - assert_that(g_sizeopt, is_equal_to(0)); + assert_that(g_window_size_type, is_equal_to(Fixed)); } Ensure(ParseGeometry, FailsOnMissingHeightVariant2) { always_expect_error_log(); - g_initial_width = g_initial_height = g_sizeopt = 0; + g_initial_width = g_initial_height = 0; assert_that(parse_geometry_string("1234x"), is_equal_to(-1)); assert_that(g_initial_width, is_equal_to(1234)); assert_that(g_initial_height, is_equal_to(0)); - assert_that(g_sizeopt, is_equal_to(0)); + assert_that(g_window_size_type, is_equal_to(Fixed)); } Ensure(ParseGeometry, HandlesPercentageOfScreen) { - g_initial_width = g_initial_height = g_sizeopt = 0; + g_initial_width = g_initial_height = 0; assert_that(parse_geometry_string("80%"), is_equal_to(0)); assert_that(g_initial_width, is_equal_to(80)); assert_that(g_initial_height, is_equal_to(80)); - assert_that(g_sizeopt, is_equal_to(-1)); + assert_that(g_window_size_type, is_equal_to(PercentageOfScreen)); } Ensure(ParseGeometry, HandlesSpecificWidthAndHeightPercentageOfScreen) { - g_initial_width = g_initial_height = g_sizeopt = 0; + g_initial_width = g_initial_height = 0; assert_that(parse_geometry_string("100%x60%"), is_equal_to(0)); assert_that(g_initial_width, is_equal_to(100)); assert_that(g_initial_height, is_equal_to(60)); - assert_that(g_sizeopt, is_equal_to(-1)); + assert_that(g_window_size_type, is_equal_to(PercentageOfScreen)); } Ensure(ParseGeometry, HandlesSpecifiedDPI) { - g_dpi = g_initial_width = g_initial_height = g_sizeopt = 0; + g_dpi = g_initial_width = g_initial_height = 0; assert_that(parse_geometry_string("1234x2345@234"), is_equal_to(0)); assert_that(g_dpi, is_equal_to(234)); assert_that(g_initial_width, is_equal_to(1234)); assert_that(g_initial_height, is_equal_to(2345)); - assert_that(g_sizeopt, is_equal_to(0)); + assert_that(g_window_size_type, is_equal_to(Fixed)); } Ensure(ParseGeometry, HandlesSpecifiedXPosition) { - g_xpos = g_ypos = g_initial_width = g_initial_height = g_sizeopt = 0; + g_xpos = g_ypos = g_initial_width = g_initial_height = 0; assert_that(parse_geometry_string("1234x2345+123"), is_equal_to(0)); @@ -105,12 +106,12 @@ Ensure(ParseGeometry, HandlesSpecifiedXPosition) assert_that(g_pos, is_equal_to(1)); assert_that(g_initial_width, is_equal_to(1234)); assert_that(g_initial_height, is_equal_to(2345)); - assert_that(g_sizeopt, is_equal_to(0)); + assert_that(g_window_size_type, is_equal_to(Fixed)); } Ensure(ParseGeometry, HandlesSpecifiedNegativeXPosition) { - g_ypos = g_xpos = g_initial_width = g_initial_height = g_sizeopt = 0; + g_ypos = g_xpos = g_initial_width = g_initial_height = 0; assert_that(parse_geometry_string("1234x2345-500"), is_equal_to(0)); @@ -119,12 +120,12 @@ Ensure(ParseGeometry, HandlesSpecifiedNegativeXPosition) assert_that(g_pos, is_equal_to(2)); assert_that(g_initial_width, is_equal_to(1234)); assert_that(g_initial_height, is_equal_to(2345)); - assert_that(g_sizeopt, is_equal_to(0)); + assert_that(g_window_size_type, is_equal_to(Fixed)); } Ensure(ParseGeometry, HandlesSpecifiedNegativeXAndYPosition) { - g_ypos = g_xpos = g_initial_width = g_initial_height = g_sizeopt = 0; + g_ypos = g_xpos = g_initial_width = g_initial_height = 0; assert_that(parse_geometry_string("1234x2345-500-501"), is_equal_to(0)); @@ -133,12 +134,12 @@ Ensure(ParseGeometry, HandlesSpecifiedNegativeXAndYPosition) assert_that(g_pos, is_equal_to(2 | 4)); assert_that(g_initial_width, is_equal_to(1234)); assert_that(g_initial_height, is_equal_to(2345)); - assert_that(g_sizeopt, is_equal_to(0)); + assert_that(g_window_size_type, is_equal_to(Fixed)); } Ensure(ParseGeometry, HandlesSpecifiedXandYPosition) { - g_xpos = g_ypos = g_initial_width = g_initial_height = g_sizeopt = 0; + g_xpos = g_ypos = g_initial_width = g_initial_height = 0; assert_that(parse_geometry_string("1234x2345+123+234"), is_equal_to(0)); @@ -147,12 +148,12 @@ Ensure(ParseGeometry, HandlesSpecifiedXandYPosition) assert_that(g_pos, is_equal_to(1)); assert_that(g_initial_width, is_equal_to(1234)); assert_that(g_initial_height, is_equal_to(2345)); - assert_that(g_sizeopt, is_equal_to(0)); + assert_that(g_window_size_type, is_equal_to(Fixed)); } Ensure(ParseGeometry, HandlesSpecifiedXandYPositionWithDPI) { - g_dpi = g_xpos = g_ypos = g_initial_width = g_initial_height = g_sizeopt = 0; + g_dpi = g_xpos = g_ypos = g_initial_width = g_initial_height = 0; assert_that(parse_geometry_string("1234x2345@678+123+234"), is_equal_to(0)); @@ -161,14 +162,14 @@ Ensure(ParseGeometry, HandlesSpecifiedXandYPositionWithDPI) assert_that(g_ypos, is_equal_to(234)); assert_that(g_initial_width, is_equal_to(1234)); assert_that(g_initial_height, is_equal_to(2345)); - assert_that(g_sizeopt, is_equal_to(0)); + assert_that(g_window_size_type, is_equal_to(Fixed)); } Ensure(ParseGeometry, HandlesSpecialNameWorkarea) { assert_that(parse_geometry_string("workarea"), is_equal_to(0)); - assert_that(g_sizeopt, is_equal_to(1)); + assert_that(g_window_size_type, is_equal_to(Workarea)); } @@ -199,10 +200,10 @@ Ensure(ParseGeometry, FailsOnMixingPixelsAndPercents) { always_expect_error_log(); - g_sizeopt = 0; + g_window_size_type = Fixed; assert_that(parse_geometry_string("1234%x2345"), is_equal_to(-1)); - g_sizeopt = 0; + g_window_size_type = Fixed; assert_that(parse_geometry_string("1234x2345%"), is_equal_to(-1)); } @@ -210,19 +211,19 @@ Ensure(ParseGeometry, FailsOnGarbageAtEndOfString) { always_expect_error_log(); - g_sizeopt = 0; + g_window_size_type = Fixed; assert_that(parse_geometry_string("1234%1239123081232345abcdefgadkfjafa4af048"), is_equal_to(-1)); - g_sizeopt = 0; + g_window_size_type = Fixed; assert_that(parse_geometry_string("1235abcer9823461"), is_equal_to(-1)); - g_sizeopt = 0; + g_window_size_type = Fixed; assert_that(parse_geometry_string("1235%x123%+123123+123123asdkjfasdf"), is_equal_to(-1)); - g_sizeopt = 0; + g_window_size_type = Fixed; assert_that(parse_geometry_string("1235%x123%@123asdkjfasdf"), is_equal_to(-1)); - g_sizeopt = 0; + g_window_size_type = Fixed; assert_that(parse_geometry_string("1235%x123%@123+1-2asdkjfasdf"), is_equal_to(-1)); } diff --git a/tests/resize_test.c b/tests/resize_test.c index 6a12ef1..ccd4ead 100644 --- a/tests/resize_test.c +++ b/tests/resize_test.c @@ -11,9 +11,9 @@ AfterEach(Resize) {}; /* globals driven by xwin.c */ RD_BOOL g_user_quit; RD_BOOL g_exit_mainloop; -int g_sizeopt; uint32 g_initial_width; uint32 g_initial_height; +window_size_type_t g_window_size_type; uint16 g_session_width; uint16 g_session_height; int g_xpos; diff --git a/tests/xwin_test.c b/tests/xwin_test.c index de6a202..77138df 100644 --- a/tests/xwin_test.c +++ b/tests/xwin_test.c @@ -17,9 +17,9 @@ AfterEach(XWIN) {}; RD_BOOL g_user_quit; RD_BOOL g_exit_mainloop; -int g_sizeopt; uint32 g_initial_width; uint32 g_initial_height; +window_size_type_t g_window_size_type; uint16 g_session_width; uint16 g_session_height; int g_xpos; diff --git a/types.h b/types.h index f2d6aef..922af8a 100644 --- a/types.h +++ b/types.h @@ -308,4 +308,11 @@ FILEINFO; typedef RD_BOOL(*str_handle_lines_t) (const char *line, void *data); +typedef enum { + Fixed, + PercentageOfScreen, + Workarea, + Fullscreen, +} window_size_type_t; + #endif /* _TYPES_H */ diff --git a/xwin.c b/xwin.c index 8602c34..3eeffd8 100644 --- a/xwin.c +++ b/xwin.c @@ -45,7 +45,7 @@ extern RD_BOOL g_user_quit; extern RD_BOOL g_exit_mainloop; -extern int g_sizeopt; +extern window_size_type_t g_window_size_type; extern uint32 g_initial_width; extern uint32 g_initial_height; extern uint16 g_session_width; @@ -1971,25 +1971,20 @@ ui_init_connection(void) /* * Determine desktop size */ - if (g_fullscreen) + if (g_fullscreen || g_window_size_type == Fullscreen) { g_initial_width = WidthOfScreen(g_screen); g_initial_height = HeightOfScreen(g_screen); g_using_full_workarea = True; } - else if (g_sizeopt < 0) + else if (g_window_size_type == PercentageOfScreen) { - /* Percent of screen */ - if (-g_sizeopt >= 100) - g_using_full_workarea = True; - /* g_initial_width/height holds percentage of screen in each axis */ g_initial_height = HeightOfScreen(g_screen) * g_initial_height / 100; g_initial_width = WidthOfScreen(g_screen) * g_initial_width / 100; } - else if (g_sizeopt == 1) + else if (g_window_size_type == Workarea) { - /* Fetch geometry from _NET_WORKAREA */ uint32 x, y, cx, cy; if (get_current_workarea(&x, &y, &cx, &cy) == 0) { @@ -2000,7 +1995,7 @@ ui_init_connection(void) else { logger(GUI, Warning, - "Failed to get workarea: probably your window manager does not support extended hints\n"); + "Failed to get workarea: probably your window manager does not support extended hints, using full screensize as fallback\n"); g_initial_width = WidthOfScreen(g_screen); g_initial_height = HeightOfScreen(g_screen); } @@ -2831,7 +2826,9 @@ xwin_process_events(void) if (xevent.xconfigure.window == DefaultRootWindow(g_display)) { /* only for fullscreen or x%-of-screen-sized windows */ - if (g_sizeopt || g_fullscreen) + if (g_window_size_type == PercentageOfScreen + || g_window_size_type == Fullscreen + || g_fullscreen) { if (xevent.xconfigure.width != WidthOfScreen(g_screen) || xevent.xconfigure.height != HeightOfScreen(g_screen)) From 8ef9f39aa815968caf39de8a549956f3d88726e5 Mon Sep 17 00:00:00 2001 From: Cendio Date: Wed, 10 Jan 2018 12:21:25 +0100 Subject: [PATCH 3/8] Avoid fight about requested session size upon disconnect/reconnect resize. Move ui_init_connection() outside main loop since it is the source for the fight of requested session size after a resize reconnect. This should only be called once for setting up initial requested session size via command line args. Signed-off-by: Henrik Andersson Signed-off-by: Thomas Nilefalk --- rdesktop.c | 3 ++- xwin.c | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/rdesktop.c b/rdesktop.c index f6c647c..be7f157 100644 --- a/rdesktop.c +++ b/rdesktop.c @@ -853,6 +853,7 @@ main(int argc, char *argv[]) break; case 'f': + g_window_size_type = Fullscreen; g_fullscreen = True; break; @@ -1296,6 +1297,7 @@ main(int argc, char *argv[]) dvc_init(); rdpedisp_init(); + ui_init_connection(); g_reconnect_loop = False; while (1) @@ -1320,7 +1322,6 @@ main(int argc, char *argv[]) g_network_error = False; } - ui_init_connection(); utils_apply_session_size_limitations(&g_initial_width, &g_initial_height); if (!rdp_connect diff --git a/xwin.c b/xwin.c index 3eeffd8..a0050a7 100644 --- a/xwin.c +++ b/xwin.c @@ -1971,7 +1971,7 @@ ui_init_connection(void) /* * Determine desktop size */ - if (g_fullscreen || g_window_size_type == Fullscreen) + if (g_window_size_type == Fullscreen) { g_initial_width = WidthOfScreen(g_screen); g_initial_height = HeightOfScreen(g_screen); From bcd676dacc4b26e8f3cec95bf44460ae30f9f1ee Mon Sep 17 00:00:00 2001 From: Cendio Date: Wed, 10 Jan 2018 15:30:50 +0100 Subject: [PATCH 4/8] Remove double definition of global variables in test Signed-off-by: Henrik Andersson Signed-off-by: Thomas Nilefalk --- tests/rdp_test.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/rdp_test.c b/tests/rdp_test.c index 6ccdc87..22a5f1a 100644 --- a/tests/rdp_test.c +++ b/tests/rdp_test.c @@ -21,8 +21,6 @@ RDP_VERSION g_rdp_version; uint16 g_server_rdp_version; uint32 g_rdp5_performanceflags; int g_server_depth; -uint32 g_initial_width; -uint32 g_initial_height; RD_BOOL g_bitmap_cache; RD_BOOL g_bitmap_cache_persist_enable; RD_BOOL g_numlock_sync; From 7f76e2218ab853205780a1b2b7521c5a9c4f3b78 Mon Sep 17 00:00:00 2001 From: Cendio Date: Wed, 10 Jan 2018 15:41:12 +0100 Subject: [PATCH 5/8] Rename of global variable for clarity Signed-off-by: Henrik Andersson Signed-off-by: Thomas Nilefalk --- rdesktop.c | 18 +++++----- rdp.c | 4 +-- secure.c | 10 +++--- tests/parse_geometry_test.c | 66 ++++++++++++++++++------------------- tests/resize_test.c | 12 +++---- tests/xwin_test.c | 4 +-- xwin.c | 30 ++++++++--------- 7 files changed, 72 insertions(+), 72 deletions(-) diff --git a/rdesktop.c b/rdesktop.c index be7f157..9ec06a2 100644 --- a/rdesktop.c +++ b/rdesktop.c @@ -69,11 +69,11 @@ int g_keyboard_subtype = 0x0; /* Defaults to US keyboard layout */ int g_keyboard_functionkeys = 0xc; /* Defaults to US keyboard layout */ int g_dpi = 0; /* device DPI: default not set */ -/* Following variables holds the initial width and height for a +/* Following variables holds the requested width and height for a rdesktop window, this is sent upon connect and tells the server what size of session we want to have. Set to decent defaults. */ -uint32 g_initial_width = 1024; -uint32 g_initial_height = 768; +uint32 g_requested_session_width = 1024; +uint32 g_requested_session_height = 768; window_size_type_t g_window_size_type = Fixed; @@ -614,7 +614,7 @@ int parse_geometry_string(const char *optarg) return -1; } - g_initial_width = value; + g_requested_session_width = value; ps = pe; /* expect % or x */ @@ -641,7 +641,7 @@ int parse_geometry_string(const char *optarg) return -1; } - g_initial_height = value; + g_requested_session_height = value; ps = pe; if (*ps == '%' && g_window_size_type == Fixed) @@ -666,7 +666,7 @@ int parse_geometry_string(const char *optarg) if (g_window_size_type == PercentageOfScreen) { /* percentage of screen used for both width and height */ - g_initial_height = g_initial_width; + g_requested_session_height = g_requested_session_width; } else { @@ -1322,7 +1322,7 @@ main(int argc, char *argv[]) g_network_error = False; } - utils_apply_session_size_limitations(&g_initial_width, &g_initial_height); + utils_apply_session_size_limitations(&g_requested_session_width, &g_requested_session_height); if (!rdp_connect (server, flags, domain, g_password, shell, directory, g_reconnect_loop)) @@ -1392,7 +1392,7 @@ main(int argc, char *argv[]) if (g_pending_resize) { logger(Core, Verbose, "Resize reconnect loop triggered, new size %dx%d", - g_initial_width, g_initial_height); + g_requested_session_width, g_requested_session_height); g_pending_resize = False; g_reconnect_loop = True; continue; @@ -1925,7 +1925,7 @@ rd_create_ui() if (!ui_have_window()) { /* create a window if we don't have one initialized */ - if (!ui_create_window(g_initial_width, g_initial_height)) + if (!ui_create_window(g_requested_session_width, g_requested_session_height)) exit(EX_OSERR); } else diff --git a/rdp.c b/rdp.c index 13e9f95..54e51d9 100644 --- a/rdp.c +++ b/rdp.c @@ -43,8 +43,8 @@ extern RDP_VERSION g_rdp_version; extern uint16 g_server_rdp_version; extern uint32 g_rdp5_performanceflags; extern int g_server_depth; -extern uint32 g_initial_width; -extern uint32 g_initial_height; +extern uint32 g_requested_session_width; +extern uint32 g_requested_session_height; extern RD_BOOL g_bitmap_cache; extern RD_BOOL g_bitmap_cache_persist_enable; extern RD_BOOL g_numlock_sync; diff --git a/secure.c b/secure.c index d5b69b1..f416aba 100644 --- a/secure.c +++ b/secure.c @@ -23,8 +23,8 @@ #include "ssl.h" extern char g_hostname[16]; -extern uint32 g_initial_width; -extern uint32 g_initial_height; +extern uint32 g_requested_session_width; +extern uint32 g_requested_session_height; extern int g_dpi; extern unsigned int g_keylayout; extern int g_keyboard_type; @@ -426,8 +426,8 @@ sec_out_mcs_connect_initial_pdu(STREAM s, uint32 selected_protocol) out_uint16_le(s, CS_CORE); /* type */ out_uint16_le(s, 216 + (g_dpi > 0 ? 18 : 0)); /* length */ out_uint32_le(s, rdpversion); /* version */ - out_uint16_le(s, g_initial_width); /* desktopWidth */ - out_uint16_le(s, g_initial_height); /* desktopHeight */ + out_uint16_le(s, g_requested_session_width); /* desktopWidth */ + out_uint16_le(s, g_requested_session_height); /* desktopHeight */ out_uint16_le(s, RNS_UD_COLOR_8BPP); /* colorDepth */ out_uint16_le(s, RNS_UD_SAS_DEL); /* SASSequence */ out_uint32_le(s, g_keylayout); /* keyboardLayout */ @@ -459,7 +459,7 @@ sec_out_mcs_connect_initial_pdu(STREAM s, uint32 selected_protocol) if (g_dpi > 0) { /* Extended client info describing monitor geometry */ - utils_calculate_dpi_scale_factors(g_initial_width, g_initial_height, g_dpi, + utils_calculate_dpi_scale_factors(g_requested_session_width, g_requested_session_height, g_dpi, &physwidth, &physheight, &desktopscale, &devicescale); out_uint32_le(s, physwidth); /* physicalwidth */ diff --git a/tests/parse_geometry_test.c b/tests/parse_geometry_test.c index bf94569..1a28222 100644 --- a/tests/parse_geometry_test.c +++ b/tests/parse_geometry_test.c @@ -26,12 +26,12 @@ RD_BOOL g_using_full_workarea; Ensure(ParseGeometry, HandlesWxH) { - g_initial_width = g_initial_height = 0; + g_requested_session_width = g_requested_session_height = 0; assert_that(parse_geometry_string("1234x2345"), is_equal_to(0)); - assert_that(g_initial_width, is_equal_to(1234)); - assert_that(g_initial_height, is_equal_to(2345)); + assert_that(g_requested_session_width, is_equal_to(1234)); + assert_that(g_requested_session_height, is_equal_to(2345)); assert_that(g_window_size_type, is_equal_to(Fixed)); } @@ -40,11 +40,11 @@ Ensure(ParseGeometry, FailsOnMissingHeight) always_expect_error_log(); - g_initial_width = g_initial_height = 0; + g_requested_session_width = g_requested_session_height = 0; assert_that(parse_geometry_string("1234"), is_equal_to(-1)); - assert_that(g_initial_width, is_equal_to(1234)); - assert_that(g_initial_height, is_equal_to(0)); + assert_that(g_requested_session_width, is_equal_to(1234)); + assert_that(g_requested_session_height, is_equal_to(0)); assert_that(g_window_size_type, is_equal_to(Fixed)); } @@ -52,116 +52,116 @@ Ensure(ParseGeometry, FailsOnMissingHeightVariant2) { always_expect_error_log(); - g_initial_width = g_initial_height = 0; + g_requested_session_width = g_requested_session_height = 0; assert_that(parse_geometry_string("1234x"), is_equal_to(-1)); - assert_that(g_initial_width, is_equal_to(1234)); - assert_that(g_initial_height, is_equal_to(0)); + assert_that(g_requested_session_width, is_equal_to(1234)); + assert_that(g_requested_session_height, is_equal_to(0)); assert_that(g_window_size_type, is_equal_to(Fixed)); } Ensure(ParseGeometry, HandlesPercentageOfScreen) { - g_initial_width = g_initial_height = 0; + g_requested_session_width = g_requested_session_height = 0; assert_that(parse_geometry_string("80%"), is_equal_to(0)); - assert_that(g_initial_width, is_equal_to(80)); - assert_that(g_initial_height, is_equal_to(80)); + assert_that(g_requested_session_width, is_equal_to(80)); + assert_that(g_requested_session_height, is_equal_to(80)); assert_that(g_window_size_type, is_equal_to(PercentageOfScreen)); } Ensure(ParseGeometry, HandlesSpecificWidthAndHeightPercentageOfScreen) { - g_initial_width = g_initial_height = 0; + g_requested_session_width = g_requested_session_height = 0; assert_that(parse_geometry_string("100%x60%"), is_equal_to(0)); - assert_that(g_initial_width, is_equal_to(100)); - assert_that(g_initial_height, is_equal_to(60)); + assert_that(g_requested_session_width, is_equal_to(100)); + assert_that(g_requested_session_height, is_equal_to(60)); assert_that(g_window_size_type, is_equal_to(PercentageOfScreen)); } Ensure(ParseGeometry, HandlesSpecifiedDPI) { - g_dpi = g_initial_width = g_initial_height = 0; + g_dpi = g_requested_session_width = g_requested_session_height = 0; assert_that(parse_geometry_string("1234x2345@234"), is_equal_to(0)); assert_that(g_dpi, is_equal_to(234)); - assert_that(g_initial_width, is_equal_to(1234)); - assert_that(g_initial_height, is_equal_to(2345)); + assert_that(g_requested_session_width, is_equal_to(1234)); + assert_that(g_requested_session_height, is_equal_to(2345)); assert_that(g_window_size_type, is_equal_to(Fixed)); } Ensure(ParseGeometry, HandlesSpecifiedXPosition) { - g_xpos = g_ypos = g_initial_width = g_initial_height = 0; + g_xpos = g_ypos = g_requested_session_width = g_requested_session_height = 0; assert_that(parse_geometry_string("1234x2345+123"), is_equal_to(0)); assert_that(g_xpos, is_equal_to(123)); assert_that(g_ypos, is_equal_to(0)); assert_that(g_pos, is_equal_to(1)); - assert_that(g_initial_width, is_equal_to(1234)); - assert_that(g_initial_height, is_equal_to(2345)); + assert_that(g_requested_session_width, is_equal_to(1234)); + assert_that(g_requested_session_height, is_equal_to(2345)); assert_that(g_window_size_type, is_equal_to(Fixed)); } Ensure(ParseGeometry, HandlesSpecifiedNegativeXPosition) { - g_ypos = g_xpos = g_initial_width = g_initial_height = 0; + g_ypos = g_xpos = g_requested_session_width = g_requested_session_height = 0; assert_that(parse_geometry_string("1234x2345-500"), is_equal_to(0)); assert_that(g_xpos, is_equal_to(-500)); assert_that(g_ypos, is_equal_to(0)); assert_that(g_pos, is_equal_to(2)); - assert_that(g_initial_width, is_equal_to(1234)); - assert_that(g_initial_height, is_equal_to(2345)); + assert_that(g_requested_session_width, is_equal_to(1234)); + assert_that(g_requested_session_height, is_equal_to(2345)); assert_that(g_window_size_type, is_equal_to(Fixed)); } Ensure(ParseGeometry, HandlesSpecifiedNegativeXAndYPosition) { - g_ypos = g_xpos = g_initial_width = g_initial_height = 0; + g_ypos = g_xpos = g_requested_session_width = g_requested_session_height = 0; assert_that(parse_geometry_string("1234x2345-500-501"), is_equal_to(0)); assert_that(g_xpos, is_equal_to(-500)); assert_that(g_ypos, is_equal_to(-501)); assert_that(g_pos, is_equal_to(2 | 4)); - assert_that(g_initial_width, is_equal_to(1234)); - assert_that(g_initial_height, is_equal_to(2345)); + assert_that(g_requested_session_width, is_equal_to(1234)); + assert_that(g_requested_session_height, is_equal_to(2345)); assert_that(g_window_size_type, is_equal_to(Fixed)); } Ensure(ParseGeometry, HandlesSpecifiedXandYPosition) { - g_xpos = g_ypos = g_initial_width = g_initial_height = 0; + g_xpos = g_ypos = g_requested_session_width = g_requested_session_height = 0; assert_that(parse_geometry_string("1234x2345+123+234"), is_equal_to(0)); assert_that(g_xpos, is_equal_to(123)); assert_that(g_ypos, is_equal_to(234)); assert_that(g_pos, is_equal_to(1)); - assert_that(g_initial_width, is_equal_to(1234)); - assert_that(g_initial_height, is_equal_to(2345)); + assert_that(g_requested_session_width, is_equal_to(1234)); + assert_that(g_requested_session_height, is_equal_to(2345)); assert_that(g_window_size_type, is_equal_to(Fixed)); } Ensure(ParseGeometry, HandlesSpecifiedXandYPositionWithDPI) { - g_dpi = g_xpos = g_ypos = g_initial_width = g_initial_height = 0; + g_dpi = g_xpos = g_ypos = g_requested_session_width = g_requested_session_height = 0; assert_that(parse_geometry_string("1234x2345@678+123+234"), is_equal_to(0)); assert_that(g_dpi, is_equal_to(678)); assert_that(g_xpos, is_equal_to(123)); assert_that(g_ypos, is_equal_to(234)); - assert_that(g_initial_width, is_equal_to(1234)); - assert_that(g_initial_height, is_equal_to(2345)); + assert_that(g_requested_session_width, is_equal_to(1234)); + assert_that(g_requested_session_height, is_equal_to(2345)); assert_that(g_window_size_type, is_equal_to(Fixed)); } diff --git a/tests/resize_test.c b/tests/resize_test.c index ccd4ead..80da9b8 100644 --- a/tests/resize_test.c +++ b/tests/resize_test.c @@ -11,8 +11,8 @@ AfterEach(Resize) {}; /* globals driven by xwin.c */ RD_BOOL g_user_quit; RD_BOOL g_exit_mainloop; -uint32 g_initial_width; -uint32 g_initial_height; +uint32 g_requested_session_width; +uint32 g_requested_session_height; window_size_type_t g_window_size_type; uint16 g_session_width; uint16 g_session_height; @@ -56,8 +56,8 @@ RDP_VERSION g_rdp_version; uint16 g_server_rdp_version; uint32 g_rdp5_performanceflags; int g_server_depth; -uint32 g_initial_width; -uint32 g_initial_height; +uint32 g_requested_session_width; +uint32 g_requested_session_height; RD_BOOL g_bitmap_cache; RD_BOOL g_bitmap_cache_persist_enable; RD_BOOL g_numlock_sync; @@ -87,8 +87,8 @@ RD_BOOL g_local_cursor; /* globals from secure.c */ char g_hostname[16]; -uint32 g_initial_width; -uint32 g_initial_height; +uint32 g_requested_session_width; +uint32 g_requested_session_height; int g_dpi; unsigned int g_keylayout; int g_keyboard_type; diff --git a/tests/xwin_test.c b/tests/xwin_test.c index 77138df..8dd1f3d 100644 --- a/tests/xwin_test.c +++ b/tests/xwin_test.c @@ -17,8 +17,8 @@ AfterEach(XWIN) {}; RD_BOOL g_user_quit; RD_BOOL g_exit_mainloop; -uint32 g_initial_width; -uint32 g_initial_height; +uint32 g_requested_session_width; +uint32 g_requested_session_height; window_size_type_t g_window_size_type; uint16 g_session_width; uint16 g_session_height; diff --git a/xwin.c b/xwin.c index a0050a7..45dc6c0 100644 --- a/xwin.c +++ b/xwin.c @@ -46,8 +46,8 @@ extern RD_BOOL g_user_quit; extern RD_BOOL g_exit_mainloop; extern window_size_type_t g_window_size_type; -extern uint32 g_initial_width; -extern uint32 g_initial_height; +extern uint32 g_requested_session_width; +extern uint32 g_requested_session_height; extern uint16 g_session_width; extern uint16 g_session_height; extern int g_xpos; @@ -1973,31 +1973,31 @@ ui_init_connection(void) */ if (g_window_size_type == Fullscreen) { - g_initial_width = WidthOfScreen(g_screen); - g_initial_height = HeightOfScreen(g_screen); + g_requested_session_width = WidthOfScreen(g_screen); + g_requested_session_height = HeightOfScreen(g_screen); g_using_full_workarea = True; } else if (g_window_size_type == PercentageOfScreen) { - /* g_initial_width/height holds percentage of screen in each axis */ - g_initial_height = HeightOfScreen(g_screen) * g_initial_height / 100; - g_initial_width = WidthOfScreen(g_screen) * g_initial_width / 100; + /* g_requested_session_width/height holds percentage of screen in each axis */ + g_requested_session_height = HeightOfScreen(g_screen) * g_requested_session_height / 100; + g_requested_session_width = WidthOfScreen(g_screen) * g_requested_session_width / 100; } else if (g_window_size_type == Workarea) { uint32 x, y, cx, cy; if (get_current_workarea(&x, &y, &cx, &cy) == 0) { - g_initial_width = cx; - g_initial_height = cy; + g_requested_session_width = cx; + g_requested_session_height = cy; g_using_full_workarea = True; } else { logger(GUI, Warning, "Failed to get workarea: probably your window manager does not support extended hints, using full screensize as fallback\n"); - g_initial_width = WidthOfScreen(g_screen); - g_initial_height = HeightOfScreen(g_screen); + g_requested_session_width = WidthOfScreen(g_screen); + g_requested_session_height = HeightOfScreen(g_screen); } } } @@ -3026,12 +3026,12 @@ process_pending_resize () * server. */ - g_initial_width = g_window_width; - g_initial_height = g_window_height; + g_requested_session_width = g_window_width; + g_requested_session_height = g_window_height; logger(GUI, Verbose, "Window resize detected, reconnecting to new size %dx%d", - g_initial_width, - g_initial_height); + g_requested_session_width, + g_requested_session_height); return True; } From 884f56a8e7de60af74a6ed4ed990af751ef8419d Mon Sep 17 00:00:00 2001 From: Cendio Date: Thu, 11 Jan 2018 10:56:59 +0100 Subject: [PATCH 6/8] Fixed failing fullscreen toggle in disconnect/reconnect mode When starting rdesktop with a fixed window size against W2008R2, toggling to fullscreen failed to resize window to expected fullscreen size. Signed-off-by: Henrik Andersson Signed-off-by: Thomas Nilefalk --- xwin.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/xwin.c b/xwin.c index 45dc6c0..bed4635 100644 --- a/xwin.c +++ b/xwin.c @@ -2368,6 +2368,8 @@ xwin_toggle_fullscreen(void) { /* Change session size using disconnect / reconnect mechanism */ g_pending_resize = True; + g_window_width = width; + g_window_height = height; return; } else From 1f3d1fb3e062e31c1c6e1b40da2cf9797e324883 Mon Sep 17 00:00:00 2001 From: Cendio Date: Thu, 11 Jan 2018 10:57:29 +0100 Subject: [PATCH 7/8] Refactor handling of user requested window sizes Extract ui_init_connection() into smaller functions to clarify purpose. Signed-off-by: Henrik Andersson Signed-off-by: Thomas Nilefalk --- proto.h | 4 +++- rdesktop.c | 29 +++++++++++++++++++++++- tests/Makefile | 2 +- tests/xwin_mock.c | 17 ++++++++++++++ xwin.c | 57 +++++++++++++++++++++-------------------------- 5 files changed, 74 insertions(+), 35 deletions(-) create mode 100644 tests/xwin_mock.c diff --git a/proto.h b/proto.h index addc6fd..7d54e93 100644 --- a/proto.h +++ b/proto.h @@ -257,7 +257,9 @@ void rdp_send_scancode(uint32 time, uint16 flags, uint8 scancode); /* xwin.c */ RD_BOOL get_key_state(unsigned int state, uint32 keysym); RD_BOOL ui_init(void); -void ui_init_connection(void); +void ui_get_screen_size(uint32 *width, uint32 *height); +void ui_get_screen_size_from_percentage(uint32 pw, uint32 ph, uint32 *width, uint32 *height); +void ui_get_workarea_size(uint32 *width, uint32 *height); void ui_deinit(void); RD_BOOL ui_create_window(uint32 width, uint32 height); void ui_resize_window(uint32 width, uint32 height); diff --git a/rdesktop.c b/rdesktop.c index 9ec06a2..298b2b6 100644 --- a/rdesktop.c +++ b/rdesktop.c @@ -729,6 +729,32 @@ int parse_geometry_string(const char *optarg) return 0; } +static void +setup_user_requested_session_size() +{ + switch(g_window_size_type) + { + case Fullscreen: + ui_get_screen_size(&g_requested_session_width, &g_requested_session_height); + break; + + case Workarea: + ui_get_workarea_size(&g_requested_session_width, &g_requested_session_height); + break; + + case Fixed: + break; + + case PercentageOfScreen: + ui_get_screen_size_from_percentage(g_requested_session_width, + g_requested_session_height, + &g_requested_session_width, + &g_requested_session_height); + break; + } +} + + /* Client program */ int main(int argc, char *argv[]) @@ -1297,7 +1323,8 @@ main(int argc, char *argv[]) dvc_init(); rdpedisp_init(); - ui_init_connection(); + + setup_user_requested_session_size(); g_reconnect_loop = False; while (1) diff --git a/tests/Makefile b/tests/Makefile index d131181..f7b39c8 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -21,7 +21,7 @@ RESIZE_MOCKS=x11_mock.o cache_mock.o xclip_mock.o xkeymap_mock.o seamless_mock.o PARSE_MOCKS=ui_mock.o rdpdr_mock.o rdpedisp_mock.o ssl_mock.o ctrl_mock.o secure_mock.o \ tcp_mock.o dvc_mock.o rdp_mock.o cache_mock.o cliprdr_mock.o disk_mock.o lspci_mock.o \ - parallel_mock.o printer_mock.o serial_mock.o xkeymap_mock.o utils_mock.o + parallel_mock.o printer_mock.o serial_mock.o xkeymap_mock.o utils_mock.o xwin_mock.o all: test diff --git a/tests/xwin_mock.c b/tests/xwin_mock.c new file mode 100644 index 0000000..6f84821 --- /dev/null +++ b/tests/xwin_mock.c @@ -0,0 +1,17 @@ +#include +#include "../rdesktop.h" + +void ui_get_screen_size(uint32 *width, uint32 *height) +{ + mock(width, height); +} + +void ui_get_screen_size_from_percentage(uint32 pw, uint32 ph, uint32 *width, uint32 *height) +{ + mock(pw, ph, width, height); +} + +void ui_get_workarea_size(uint32 *width, uint32 *height) +{ + mock(width, height); +} diff --git a/xwin.c b/xwin.c index bed4635..07e4932 100644 --- a/xwin.c +++ b/xwin.c @@ -1961,44 +1961,37 @@ ui_init(void) return True; } - -/* - Initialize connection specific data, such as initial session size. - */ void -ui_init_connection(void) +ui_get_screen_size(uint32 *width, uint32 *height) { - /* - * Determine desktop size - */ - if (g_window_size_type == Fullscreen) + *width = WidthOfScreen(g_screen); + *height = HeightOfScreen(g_screen); +} + +void +ui_get_screen_size_from_percentage(uint32 pw, uint32 ph, uint32 *width, uint32 *height) +{ + uint32 sw,sh; + ui_get_screen_size(&sw, &sh); + *width = sw * pw / 100; + *height = sh * ph / 100; +} + +void +ui_get_workarea_size(uint32 *width, uint32 *height) +{ + uint32 x, y, w, h; + if (get_current_workarea(&x, &y, &w, &h) == 0) { - g_requested_session_width = WidthOfScreen(g_screen); - g_requested_session_height = HeightOfScreen(g_screen); + *width = w; + *height = h; g_using_full_workarea = True; } - else if (g_window_size_type == PercentageOfScreen) + else { - /* g_requested_session_width/height holds percentage of screen in each axis */ - g_requested_session_height = HeightOfScreen(g_screen) * g_requested_session_height / 100; - g_requested_session_width = WidthOfScreen(g_screen) * g_requested_session_width / 100; - } - else if (g_window_size_type == Workarea) - { - uint32 x, y, cx, cy; - if (get_current_workarea(&x, &y, &cx, &cy) == 0) - { - g_requested_session_width = cx; - g_requested_session_height = cy; - g_using_full_workarea = True; - } - else - { - logger(GUI, Warning, - "Failed to get workarea: probably your window manager does not support extended hints, using full screensize as fallback\n"); - g_requested_session_width = WidthOfScreen(g_screen); - g_requested_session_height = HeightOfScreen(g_screen); - } + logger(GUI, Warning, + "Failed to get workarea: probably your window manager does not support extended hints, using full screensize as fallback\n"); + ui_get_screen_size(width, height); } } From a6d82619ee7ef5c1853869e56315e80a10d76f9b Mon Sep 17 00:00:00 2001 From: Cendio Date: Thu, 11 Jan 2018 15:00:46 +0100 Subject: [PATCH 8/8] Fix regression introduced in commit 85c10b5 There was either an X11 BadMatch error crash or the rdesktop main window disappeared when toggling from window to fullscreen. The bug is consitently reproducible but only on some systems, maybe this is a X11 version dependent bug. Move back to old beahavior were we destroy and recreate the window when toggling between fullscreen and windowed mode. Signed-off-by: Henrik Andersson --- xwin.c | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/xwin.c b/xwin.c index 07e4932..c538ed9 100644 --- a/xwin.c +++ b/xwin.c @@ -2287,8 +2287,6 @@ xwin_toggle_fullscreen(void) { uint32 x, y, width, height; XWindowAttributes attr; - XSetWindowAttributes setattr; - unsigned long value_mask; Pixmap contents = 0; Window unused; int dest_x, dest_y; @@ -2347,14 +2345,10 @@ xwin_toggle_fullscreen(void) } /* Resize rdesktop window using new size and window attributes */ - XUnmapWindow(g_display, g_wnd); - - XMoveResizeWindow(g_display, g_wnd, x, y, width, height); - - value_mask = get_window_attribs(&setattr); - XChangeWindowAttributes(g_display, g_wnd, value_mask, &setattr); - - XMapWindow(g_display, g_wnd); + g_xpos = x; + g_ypos = y; + ui_destroy_window(); + ui_create_window(width, height); /* Change session size to match new window size */ if (rdpedisp_is_available() == False)