From 8589ae19de1d5699a586f96fe34a35c586e1e29c Mon Sep 17 00:00:00 2001 From: random human Date: Fri, 31 Aug 2018 16:11:46 +0530 Subject: Fix bugs listed by clang's static analyzer A few pedantic changes and unused variables (1-4), and genuine bugs (5, 6). The reports with the corresponding files and lines numbers are as follows. 1. backend/libinput/tablet_pad.c@31,44,57 "Allocator sizeof operand mismatch" "Result of 'calloc' is converted to a pointer of type 'unsigned int', which is incompatible with sizeof operand type 'int'" 2. types/tablet_v2/wlr_tablet_v2_pad.c@371 "Allocator sizeof operand mismatch" "Result of 'calloc' is converted to a pointer of type 'uint32_t', which is incompatible with sizeof operand type 'int'" 3. types/wlr_cursor.c@335 "Dead initialization" "Value stored to 'dx'/'dy' during its initialization is never read" 4. rootston/xdg_shell.c@510 "Dead initialization" "Value stored to 'desktop' during its initialization is never read" 5. types/tablet_v2/wlr_tablet_v2_pad.c@475 "Dereference of null pointer" "Access to field 'strips' results in a dereference of a null pointer (loaded from field 'current_client')" The boolean logic was incorrect (c.f. the check in the following function). 6. examples/idle.c@163,174,182 "Uninitialized argument value" "1st function call argument is an uninitialized value" If close_timeout != 0, but simulate_activity_timeout >= close_timeout, the program would segfault at pthread_cancel(t1). --- backend/libinput/tablet_pad.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'backend') diff --git a/backend/libinput/tablet_pad.c b/backend/libinput/tablet_pad.c index 579a11cf..0642f925 100644 --- a/backend/libinput/tablet_pad.c +++ b/backend/libinput/tablet_pad.c @@ -28,7 +28,7 @@ static void add_pad_group_from_libinput(struct wlr_tablet_pad *pad, ++group->ring_count; } } - group->rings = calloc(sizeof(int), group->ring_count); + group->rings = calloc(sizeof(unsigned int), group->ring_count); size_t ring = 0; for (size_t i = 0; i < pad->ring_count; ++i) { if (libinput_tablet_pad_mode_group_has_ring(li_group, i)) { @@ -41,7 +41,7 @@ static void add_pad_group_from_libinput(struct wlr_tablet_pad *pad, ++group->strip_count; } } - group->strips = calloc(sizeof(int), group->strip_count); + group->strips = calloc(sizeof(unsigned int), group->strip_count); size_t strip = 0; for (size_t i = 0; i < pad->strip_count; ++i) { if (libinput_tablet_pad_mode_group_has_strip(li_group, i)) { @@ -54,7 +54,7 @@ static void add_pad_group_from_libinput(struct wlr_tablet_pad *pad, ++group->button_count; } } - group->buttons = calloc(sizeof(int), group->button_count); + group->buttons = calloc(sizeof(unsigned int), group->button_count); size_t button = 0; for (size_t i = 0; i < pad->button_count; ++i) { if (libinput_tablet_pad_mode_group_has_button(li_group, i)) { -- cgit v1.2.3 From e84f01168d55087932529301bb8e1c723244b72b Mon Sep 17 00:00:00 2001 From: emersion Date: Sat, 1 Sep 2018 23:43:16 +0200 Subject: backend/drm: allow disabling outputs in NEEDS_MODESET state This correctly frees CRTCs when disabling outputs without setting a mode. --- backend/drm/drm.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'backend') diff --git a/backend/drm/drm.c b/backend/drm/drm.c index a666ce71..c1fa1992 100644 --- a/backend/drm/drm.c +++ b/backend/drm/drm.c @@ -344,7 +344,8 @@ static void drm_connector_start_renderer(struct wlr_drm_connector *conn) { void enable_drm_connector(struct wlr_output *output, bool enable) { struct wlr_drm_connector *conn = (struct wlr_drm_connector *)output; - if (conn->state != WLR_DRM_CONN_CONNECTED) { + if (conn->state != WLR_DRM_CONN_CONNECTED + && conn->state != WLR_DRM_CONN_NEEDS_MODESET) { return; } -- cgit v1.2.3 From ef88df214249b578b030e0cdd02153fde2bba848 Mon Sep 17 00:00:00 2001 From: emersion Date: Sun, 2 Sep 2018 01:03:20 +0200 Subject: backend/drm: emit new_output after scanning connectors This prevents receiving modesetting requests from the compositor while we don't have the whole picture (ie. while we haven't yet scanned all connectors). This also makes connectors without CRTCs disabled (they can't be enabled yet even if some CRTCs are free'd -- this is future work). --- backend/drm/drm.c | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) (limited to 'backend') diff --git a/backend/drm/drm.c b/backend/drm/drm.c index c1fa1992..88055f54 100644 --- a/backend/drm/drm.c +++ b/backend/drm/drm.c @@ -564,7 +564,7 @@ static bool drm_connector_set_mode(struct wlr_output *output, struct wlr_drm_crtc *crtc = conn->crtc; if (!crtc) { wlr_log(WLR_ERROR, "Unable to match %s with a CRTC", conn->output.name); - goto error_conn; + return false; } wlr_log(WLR_DEBUG, "%s: crtc=%td ovr=%td pri=%td cur=%td", conn->output.name, crtc - drm->crtcs, @@ -864,11 +864,13 @@ void scan_drm_connectors(struct wlr_drm_backend *drm) { return; } - size_t seen_len = wl_list_length(&drm->outputs); + size_t drm_outputs_len = wl_list_length(&drm->outputs); // +1 so length can never be 0, which is undefined behaviour. // Last element isn't used. - bool seen[seen_len + 1]; - memset(seen, 0, sizeof(seen)); + bool seen[drm_outputs_len + 1]; + memset(seen, false, sizeof(seen)); + size_t new_outputs_len = 0; + struct wlr_drm_connector *new_outputs[drm_outputs_len + 1]; for (int i = 0; i < res->count_connectors; ++i) { drmModeConnector *drm_conn = drmModeGetConnector(drm->fd, @@ -974,13 +976,10 @@ void scan_drm_connectors(struct wlr_drm_backend *drm) { wl_list_insert(&wlr_conn->output.modes, &mode->wlr_mode.link); } - wlr_output_update_enabled(&wlr_conn->output, true); + wlr_output_update_enabled(&wlr_conn->output, wlr_conn->crtc != NULL); wlr_conn->state = WLR_DRM_CONN_NEEDS_MODESET; - wlr_log(WLR_INFO, "Sending modesetting signal for '%s'", - wlr_conn->output.name); - wlr_signal_emit_safe(&drm->backend.events.new_output, - &wlr_conn->output); + new_outputs[new_outputs_len++] = wlr_conn; } else if (wlr_conn->state == WLR_DRM_CONN_CONNECTED && drm_conn->connection != DRM_MODE_CONNECTED) { wlr_log(WLR_INFO, "'%s' disconnected", wlr_conn->output.name); @@ -999,7 +998,7 @@ void scan_drm_connectors(struct wlr_drm_backend *drm) { size_t index = wl_list_length(&drm->outputs); wl_list_for_each_safe(conn, tmp_conn, &drm->outputs, link) { index--; - if (index >= seen_len || seen[index]) { + if (index >= drm_outputs_len || seen[index]) { continue; } @@ -1011,6 +1010,15 @@ void scan_drm_connectors(struct wlr_drm_backend *drm) { wl_list_remove(&conn->link); free(conn); } + + for (size_t i = 0; i < new_outputs_len; ++i) { + struct wlr_drm_connector *conn = new_outputs[i]; + + wlr_log(WLR_INFO, "Requesting modeset for '%s'", + conn->output.name); + wlr_signal_emit_safe(&drm->backend.events.new_output, + &conn->output); + } } static void page_flip_handler(int fd, unsigned seq, -- cgit v1.2.3 From 95d05acda511e8559ab87a3d8956ee942ca1999e Mon Sep 17 00:00:00 2001 From: emersion Date: Sun, 2 Sep 2018 09:00:21 +0200 Subject: backend/drm: fix invalid VLA size in scan_drm_connectors I failed to see this issue with Valgrind because of the +1. --- backend/drm/drm.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'backend') diff --git a/backend/drm/drm.c b/backend/drm/drm.c index 88055f54..5396dcd4 100644 --- a/backend/drm/drm.c +++ b/backend/drm/drm.c @@ -864,13 +864,13 @@ void scan_drm_connectors(struct wlr_drm_backend *drm) { return; } - size_t drm_outputs_len = wl_list_length(&drm->outputs); + size_t seen_len = wl_list_length(&drm->outputs); // +1 so length can never be 0, which is undefined behaviour. // Last element isn't used. - bool seen[drm_outputs_len + 1]; + bool seen[seen_len + 1]; memset(seen, false, sizeof(seen)); size_t new_outputs_len = 0; - struct wlr_drm_connector *new_outputs[drm_outputs_len + 1]; + struct wlr_drm_connector *new_outputs[res->count_connectors + 1]; for (int i = 0; i < res->count_connectors; ++i) { drmModeConnector *drm_conn = drmModeGetConnector(drm->fd, @@ -998,7 +998,7 @@ void scan_drm_connectors(struct wlr_drm_backend *drm) { size_t index = wl_list_length(&drm->outputs); wl_list_for_each_safe(conn, tmp_conn, &drm->outputs, link) { index--; - if (index >= drm_outputs_len || seen[index]) { + if (index >= seen_len || seen[index]) { continue; } -- cgit v1.2.3