xrt: Some clang tidy fixes.

A few suppressions as well where clang-tidy did the wrong thing.

.clang-tidy

@@ -2,7 +2,7 @@
 # SPDX-License-Identifier: CC0-1.0
 # SPDX-FileCopyrightText: 2018-2020 Collabora, Ltd. and the Monado contributors
 # Ideally we'd turn back on a few of these that are disabled near the end
-Checks:          'clang-diagnostic-*,clang-analyzer-*,performance-*,bugprone-*,cert-*,readability-*,misc-*,-modernize-*,-clang-analyzer-security.insecureAPI.strcpy,-bugprone-macro-parentheses,-readability-braces-around-statements,-misc-unused-parameters,-readability-implicit-bool-conversion,-clang-diagnostic-missing-field-initializers,-clang-diagnostic-missing-braces'
+Checks:          'clang-diagnostic-*,clang-analyzer-*,performance-*,bugprone-*,cert-*,readability-*,misc-*,-modernize-*,-clang-analyzer-security.insecureAPI.strcpy,-bugprone-macro-parentheses,-readability-braces-around-statements,-misc-unused-parameters,-readability-implicit-bool-conversion,-clang-diagnostic-missing-field-initializers,-clang-diagnostic-missing-braces,-readability-uppercase-literal-suffix'
 WarningsAsErrors: ''
 HeaderFilterRegex: 'src/xrt/.*'
 AnalyzeTemporaryDtors: false

src/xrt/auxiliary/math/m_imu_3dof.c

@@ -126,11 +126,11 @@ gravity_correction(struct m_imu_3dof *f,
 		 */
 		float correction_radians = 0.5 * gyro_length * max_radians;
 		// Clamp to the range [min_radians, max_radians]
-		correction_radians = fmax(min_radians, correction_radians);
+		correction_radians = fmaxf(min_radians, correction_radians);
-		correction_radians = fmin(max_radians, correction_radians);
+		correction_radians = fminf(max_radians, correction_radians);
 		// Do not exceed the remaining error to correct for
 		correction_radians =
-		    -fmin(correction_radians, f->grav.error_angle);
+		    -fminf(correction_radians, f->grav.error_angle);
 
 		// Update how much is left.
 		f->grav.error_angle += correction_radians;

src/xrt/auxiliary/os/os_ble_dbus.c

@@ -453,7 +453,8 @@ gatt_iface_get_flag_notifiable(const DBusMessageIter *iface_elm, bool *out_bool)
 	if (ret < 0) {
 		// Error
 		return ret;
-	} else if (ret > 0) {
+	}
+	if (ret > 0) {
 		// Found the notify field!
 		*out_bool = true;
 	}
@@ -744,7 +745,8 @@ init_ble_notify(const char *dev_uuid,
 	                            dbus_address, sizeof(dbus_address));
 	if (written == 0) {
 		return -1;
-	} else if (written < 0) {
+	}
+	if (written < 0) {
 		return -1;
 	}
 
@@ -773,7 +775,8 @@ init_ble_notify(const char *dev_uuid,
 		int type = dbus_message_iter_get_arg_type(&args);
 		if (type == DBUS_TYPE_INVALID) {
 			break;
-		} else if (type == DBUS_TYPE_STRING) {
+		}
+		if (type == DBUS_TYPE_STRING) {
 			dbus_message_iter_get_basic(&args, &response);
 			printf("DBus call returned message: %s\n", response);
 		} else if (type == DBUS_TYPE_UNIX_FD) {

src/xrt/auxiliary/tracking/t_calibration.cpp

@@ -20,6 +20,7 @@
 
 #include <opencv2/opencv.hpp>
 #include <sys/stat.h>
+#include <utility>
 
 
 DEBUG_GET_ONCE_BOOL_OPTION(hsv_filter, "T_DEBUG_HSV_FILTER", false)
@@ -1380,7 +1381,7 @@ populateCacheMats(cv::Size size,
 }
 
 NormalizedCoordsCache::NormalizedCoordsCache(
-    cv::Size size,
+    cv::Size size, // NOLINT // small, pass by value
     const cv::Matx33d &intrinsics,
     const cv::Matx<double, 5, 1> &distortion)
 {
@@ -1394,7 +1395,7 @@ NormalizedCoordsCache::NormalizedCoordsCache(
 	populateCacheMats(size, inputCoords, outputCoords, cacheX_, cacheY_);
 }
 NormalizedCoordsCache::NormalizedCoordsCache(
-    cv::Size size,
+    cv::Size size, // NOLINT // small, pass by value
     const cv::Matx33d &intrinsics,
     const cv::Matx<double, 5, 1> &distortion,
     const cv::Matx33d &rectification,
@@ -1412,7 +1413,7 @@ NormalizedCoordsCache::NormalizedCoordsCache(
 }
 
 NormalizedCoordsCache::NormalizedCoordsCache(
-    cv::Size size,
+    cv::Size size, // NOLINT // small, pass by value
     const cv::Matx33d &intrinsics,
     const cv::Matx<double, 5, 1> &distortion,
     const cv::Matx33d &rectification,
@@ -1428,9 +1429,10 @@ NormalizedCoordsCache::NormalizedCoordsCache(
 
 	populateCacheMats(size, inputCoords, outputCoords, cacheX_, cacheY_);
 }
-NormalizedCoordsCache::NormalizedCoordsCache(cv::Size size,
+NormalizedCoordsCache::NormalizedCoordsCache(
-                                             const cv::Mat &intrinsics,
+    cv::Size size, // NOLINT // small, pass by value
-                                             const cv::Mat &distortion)
+    const cv::Mat &intrinsics,
+    const cv::Mat &distortion)
 {
 	std::vector<cv::Vec2f> outputCoords;
 	std::vector<cv::Vec2f> inputCoords =
@@ -1443,7 +1445,9 @@ NormalizedCoordsCache::NormalizedCoordsCache(cv::Size size,
 }
 
 cv::Vec2f
-NormalizedCoordsCache::getNormalizedImageCoords(cv::Point2f origCoords) const
+NormalizedCoordsCache::getNormalizedImageCoords(
+    // NOLINTNEXTLINE // small, pass by value
+    cv::Point2f origCoords) const
 {
 	/*
 	 * getRectSubPix is more strict than the docs would imply:
@@ -1465,7 +1469,7 @@ cv::Vec3f
 NormalizedCoordsCache::getNormalizedVector(cv::Point2f origCoords) const
 {
 	// cameras traditionally look along -z, so we want negative sqrt
-	auto pt = getNormalizedImageCoords(origCoords);
+	auto pt = getNormalizedImageCoords(std::move(origCoords));
 	auto z = -std::sqrt(1.f - pt.dot(pt));
 	return {pt[0], pt[1], z};
 }

src/xrt/auxiliary/util/u_bitwise.c

@@ -7,18 +7,20 @@
  * @author Pete Black <pete.black@collabora.com>
  * @ingroup aux_util
  */
-#include <stdio.h>
 #include "util/u_bitwise.h"
 
+#include <stdio.h>
+#include <limits.h>
+
 int
-get_bit(unsigned char *b, int num)
+get_bit(const unsigned char *b, int num)
 {
-	int index = num / 8;
+	int index = num / CHAR_BIT;
-	return (b[index] >> (7 - (num % 8))) & 1;
+	return (b[index] >> ((CHAR_BIT - 1) - (num % CHAR_BIT))) & 1;
 }
 
 int
-get_bits(unsigned char *b, int start, int num)
+get_bits(const unsigned char *b, int start, int num)
 {
 	int ret = 0;
 	for (int i = 0; i < num; i++) {
@@ -29,7 +31,10 @@ get_bits(unsigned char *b, int start, int num)
 }
 
 int
-sign_extend_13(unsigned int i)
+sign_extend_13(uint32_t i)
 {
-	return ((int)(i << 19)) >> 19;
+	static const size_t incoming_int_width = 13;
+	static const size_t adjustment =
+	    (sizeof(i) * CHAR_BIT) - incoming_int_width;
+	return ((int)(i << adjustment)) >> adjustment;
 }

src/xrt/auxiliary/util/u_bitwise.h

@@ -10,18 +10,24 @@
 
 #pragma once
 
+#include <stdint.h>
+
 #ifdef __cplusplus
 extern "C" {
 #endif
 
 int
-get_bit(unsigned char *b, int num);
+get_bit(const unsigned char *b, int num);
 
 int
-get_bits(unsigned char *b, int start, int num);
+get_bits(const unsigned char *b, int start, int num);
 
+/*!
+ * Interpret the least-significant 13 bits as a signed 13-bit integer, and cast
+ * it to a signed int for normal usage.
+ */
 int
-sign_extend_13(unsigned int i);
+sign_extend_13(uint32_t i);
 
 
 #ifdef __cplusplus

src/xrt/auxiliary/util/u_file.c

@@ -55,12 +55,12 @@ u_file_get_config_dir(char *out_path, size_t out_path_size)
 	const char *home = getenv("HOME");
 	if (xgd_home != NULL) {
 		return snprintf(out_path, out_path_size, "%s/monado", xgd_home);
-	} else if (home != NULL) {
+	}
+	if (home != NULL) {
 		return snprintf(out_path, out_path_size, "%s/.config/monado",
 		                home);
-	} else {
-		return -1;
 	}
+	return -1;
 }
 
 ssize_t

src/xrt/auxiliary/util/u_format.c

@@ -76,7 +76,7 @@ u_format_block_width(enum xrt_format f)
 	case XRT_FORMAT_BITMAP_8X8:
 	case XRT_FORMAT_BITMAP_8X1:
 		// Eight pixels per block.
-		return 8;
+		return 8; // NOLINT
 	default: assert(!"unsupported format"); return 0;
 	}
 }

src/xrt/auxiliary/util/u_json.c

@@ -51,13 +51,12 @@ u_json_get_string_into_array(const cJSON *json, char *out_str, size_t max_size)
 	if (ret < 0) {
 		U_LOG_E("Printing string failed: %d", ret);
 		return false;
-	} else if ((size_t)ret < max_size) {
-		return true;
-	} else {
-		U_LOG_E("String size %d is bigger than available %zu", ret,
-		        max_size);
-		return false;
 	}
+	if ((size_t)ret < max_size) {
+		return true;
+	}
+	U_LOG_E("String size %d is bigger than available %zu", ret, max_size);
+	return false;
 }
 
 bool

src/xrt/auxiliary/vk/vk_image_allocator.c

@@ -216,9 +216,8 @@ vk_ic_from_natives(struct vk_bundle *vk,
 		if (ret != VK_SUCCESS) {
 			close(fd);
 			break;
-		} else {
-			native_images[i].fd = fd;
 		}
+		native_images[i].fd = fd;
 	}
 #endif
 

src/xrt/compositor/main/comp_compositor.c

@@ -216,7 +216,7 @@ compositor_wait_frame(struct xrt_compositor *xc,
 		int64_t render_time_ns =
 		    c->expected_app_duration_ns + c->frame_overhead_ns;
 		int64_t swap_interval =
-		    ceil((float)render_time_ns / interval_ns);
+		    ceilf((float)render_time_ns / interval_ns);
 		int64_t render_interval_ns = swap_interval * interval_ns;
 		int64_t next_display_time =
 		    c->last_next_display_time + render_interval_ns;
@@ -475,6 +475,7 @@ compositor_poll_events(struct xrt_compositor *xc,
 #define GET_DEV_PROC(c, name)                                                  \
 	(PFN_##name) c->vk.vkGetDeviceProcAddr(c->vk.device, #name);
 
+// NOLINTNEXTLINE // don't remove the forward decl.
 VKAPI_ATTR PFN_vkVoidFunction VKAPI_CALL
 vkGetInstanceProcAddr(VkInstance instance, const char *pName);
 

src/xrt/compositor/main/comp_renderer.c

@@ -478,9 +478,8 @@ get_image_view(struct comp_swapchain_image *image,
 {
 	if (flags & XRT_LAYER_COMPOSITION_BLEND_TEXTURE_SOURCE_ALPHA_BIT) {
 		return image->views.alpha[array_index];
-	} else {
-		return image->views.no_alpha[array_index];
 	}
+	return image->views.no_alpha[array_index];
 }
 void
 comp_renderer_set_quad_layer(struct comp_renderer *r,

src/xrt/compositor/main/comp_swapchain.c

@@ -43,9 +43,8 @@ swapchain_acquire_image(struct xrt_swapchain *xsc, uint32_t *out_index)
 	int res = u_index_fifo_pop(&sc->fifo, out_index);
 	if (res >= 0) {
 		return XRT_SUCCESS;
-	} else {
-		return XRT_ERROR_NO_IMAGE_AVAILABLE;
 	}
+	return XRT_ERROR_NO_IMAGE_AVAILABLE;
 }
 
 static xrt_result_t
@@ -70,10 +69,9 @@ swapchain_release_image(struct xrt_swapchain *xsc, uint32_t index)
 
 	if (res >= 0) {
 		return XRT_SUCCESS;
-	} else {
-		// FIFO full
-		return XRT_ERROR_NO_IMAGE_AVAILABLE;
 	}
+	// FIFO full
+	return XRT_ERROR_NO_IMAGE_AVAILABLE;
 }
 
 

src/xrt/drivers/north_star/distortion/deformation_northstar.cpp

@@ -109,7 +109,7 @@ OpticalSystem::RegenerateMesh()
 }
 
 Vector2
-OpticalSystem::RenderUVToDisplayUV(Vector2 inputUV)
+OpticalSystem::RenderUVToDisplayUV(const Vector2 &inputUV)
 {
 	Vector3 rayDir;
 	ViewportPointToRayDirection(inputUV, eyePosition, clipToWorld, rayDir);
@@ -118,7 +118,7 @@ OpticalSystem::RenderUVToDisplayUV(Vector2 inputUV)
 }
 
 Vector2
-OpticalSystem::RenderUVToDisplayUV(Vector3 inputUV)
+OpticalSystem::RenderUVToDisplayUV(const Vector3 &inputUV)
 {
 
 	Vector3 sphereSpaceRayOrigin =
@@ -188,7 +188,7 @@ OpticalSystem::RenderUVToDisplayUV(Vector3 inputUV)
 }
 
 Vector2
-OpticalSystem::SolveDisplayUVToRenderUV(Vector2 inputUV,
+OpticalSystem::SolveDisplayUVToRenderUV(const Vector2 &inputUV,
                                         Vector2 initailGuess,
                                         int iterations)
 {

src/xrt/drivers/north_star/distortion/deformation_northstar.h

@@ -26,13 +26,13 @@ public:
 	}
 
 	Vector2
-	RenderUVToDisplayUV(Vector3 inputUV);
+	RenderUVToDisplayUV(const Vector3 &inputUV);
 
 	Vector2
-	RenderUVToDisplayUV(Vector2 inputUV);
+	RenderUVToDisplayUV(const Vector2 &inputUV);
 
 	Vector2
-	SolveDisplayUVToRenderUV(Vector2 inputUV,
+	SolveDisplayUVToRenderUV(const Vector2 &inputUV,
 	                         Vector2 initailGuess,
 	                         int iterations);
 

src/xrt/drivers/north_star/ns_hmd.h

@@ -183,8 +183,8 @@ ns_mesh(struct u_uv_generator *gen)
  * @ingroup drv_ns
  */
 void
-ns_display_uv_to_render_uv(struct ns_uv display_uv,
+ns_display_uv_to_render_uv(struct ns_uv in,
-                           struct ns_uv *render_uv,
+                           struct ns_uv *out,
                            struct ns_eye *eye);
 
 struct ns_optical_system *

src/xrt/ipc/ipc_server_process.c

@@ -108,7 +108,8 @@ init_tracking_origins(struct ipc_server *s)
 			if (s->xtracks[index] == NULL) {
 				s->xtracks[index] = xtrack;
 				break;
-			} else if (s->xtracks[index] == xtrack) {
+			}
+			if (s->xtracks[index] == xtrack) {
 				break;
 			}
 		}
@@ -664,7 +665,8 @@ _overlay_sort_func(const void *a, const void *b)
 	struct _z_sort_data *ob = (struct _z_sort_data *)b;
 	if (oa->z_order < ob->z_order) {
 		return -1;
-	} else if (oa->z_order > ob->z_order) {
+	}
+	if (oa->z_order > ob->z_order) {
 		return 1;
 	}
 	return 0;
@@ -960,7 +962,6 @@ update_server_state(struct ipc_server *s)
 	s->last_active_client_index = s->active_client_index;
 
 	os_mutex_unlock(&s->global_state_lock);
-	return;
 }
 
 int

src/xrt/state_trackers/oxr/oxr_api_system.c

@@ -292,6 +292,7 @@ oxr_xrGetVulkanDeviceExtensionsKHR(XrInstance instance,
 	                              namesCountOutput, namesString);
 }
 
+// NOLINTNEXTLINE // don't remove the forward decl.
 VKAPI_ATTR PFN_vkVoidFunction VKAPI_CALL
 vkGetInstanceProcAddr(VkInstance instance, const char *pName);
 

src/xrt/state_trackers/oxr/oxr_binding.c

@@ -125,9 +125,8 @@ interaction_profile_find_or_create(struct oxr_logger *log,
 		                       strlen(templ->path), &t_path);
 		if (t_path == path) {
 			break;
-		} else {
-			templ = NULL;
 		}
+		templ = NULL;
 	}
 
 	if (templ == NULL) {

src/xrt/state_trackers/oxr/oxr_input.c

@@ -75,13 +75,6 @@ oxr_action_bind_inputs(struct oxr_logger *log,
                        struct oxr_interaction_profile *profile,
                        enum oxr_sub_action_path sub_path);
 
-static void
-oxr_session_get_action_set_attachment(
-    struct oxr_session *sess,
-    XrActionSet actionSet,
-    struct oxr_action_set_attachment **act_set_attached,
-    struct oxr_action_set **act_set);
-
 /*
  *
  * Action attachment functions
@@ -905,8 +898,8 @@ oxr_input_combine_input(struct oxr_session *sess,
 			break;
 		case XRT_INPUT_TYPE_VEC1_MINUS_ONE_TO_ONE:
 		case XRT_INPUT_TYPE_VEC1_ZERO_TO_ONE:
-			if (fabs(transformed.value.vec1.x) >
+			if (fabsf(transformed.value.vec1.x) >
-			    fabs(res.value.vec1.x)) {
+			    fabsf(res.value.vec1.x)) {
 				res.value.vec1.x = transformed.value.vec1.x;
 				res_timestamp = input->timestamp;
 			}
@@ -923,7 +916,9 @@ oxr_input_combine_input(struct oxr_session *sess,
 				res_timestamp = input->timestamp;
 			}
 		} break;
-		case XRT_INPUT_TYPE_VEC3_MINUS_ONE_TO_ONE: break;
+		case XRT_INPUT_TYPE_VEC3_MINUS_ONE_TO_ONE:
+			// OpenXR has no vec3 right now.
+			break;
 		case XRT_INPUT_TYPE_POSE:
 			// shouldn't be possible to get here
 			break;
@@ -1127,7 +1122,8 @@ oxr_action_attachment_update(struct oxr_logger *log,
 		break;
 	}
 	case XR_ACTION_TYPE_FLOAT_INPUT: {
-		float value = -2.0;
+		// Smaller than any possible real value
+		float value = -2.0f; // NOLINT
 		OXR_FOR_EACH_VALID_SUBACTION_PATH(VEC1_CHECK)
 
 		changed = last.value.vec1.x != value;
@@ -1135,9 +1131,9 @@ oxr_action_attachment_update(struct oxr_logger *log,
 		break;
 	}
 	case XR_ACTION_TYPE_VECTOR2F_INPUT: {
-		float x = 0.0;
+		float x = 0.0f;
-		float y = 0.0;
+		float y = 0.0f;
-		float distance = -1.0;
+		float distance = -1.0f;
 		OXR_FOR_EACH_VALID_SUBACTION_PATH(VEC2_CHECK)
 
 		changed = (last.value.vec2.x != x) || (last.value.vec2.y != y);

src/xrt/state_trackers/oxr/oxr_space.c

@@ -221,7 +221,8 @@ oxr_space_ref_relation(struct oxr_logger *log,
 			out_relation->relation_flags =
 			    XRT_SPACE_RELATION_BITMASK_NONE;
 			return XR_SUCCESS;
-		} else if (space == XR_REFERENCE_SPACE_TYPE_STAGE) {
+		}
+		if (space == XR_REFERENCE_SPACE_TYPE_STAGE) {
 			// device poses are already in stage = "global" space
 		} else if (space == XR_REFERENCE_SPACE_TYPE_LOCAL) {
 			global_to_local_space(sess, &out_relation->pose);

src/xrt/state_trackers/oxr/oxr_swapchain.c

@@ -42,7 +42,8 @@ oxr_swapchain_acquire_image(struct oxr_logger *log,
 	if (res == XRT_ERROR_IPC_FAILURE) {
 		return oxr_error(log, XR_ERROR_INSTANCE_LOST,
 		                 "Call to xsc->acquire_image failed");
-	} else if (res != XRT_SUCCESS) {
+	}
+	if (res != XRT_SUCCESS) {
 		return oxr_error(log, XR_ERROR_RUNTIME_FAILURE,
 		                 "Call to xsc->acquire_image failed");
 	}
@@ -94,7 +95,8 @@ oxr_swapchain_wait_image(struct oxr_logger *log,
 	if (res == XRT_ERROR_IPC_FAILURE) {
 		return oxr_error(log, XR_ERROR_INSTANCE_LOST,
 		                 "Call to xsc->wait_image failed");
-	} else if (res != XRT_SUCCESS) {
+	}
+	if (res != XRT_SUCCESS) {
 		return oxr_error(log, XR_ERROR_RUNTIME_FAILURE,
 		                 "Call to xsc->wait_image failed");
 	}
@@ -125,7 +127,8 @@ oxr_swapchain_release_image(struct oxr_logger *log,
 	if (res == XRT_ERROR_IPC_FAILURE) {
 		return oxr_error(log, XR_ERROR_INSTANCE_LOST,
 		                 "Call to xsc->release_image failed");
-	} else if (res != XRT_SUCCESS) {
+	}
+	if (res != XRT_SUCCESS) {
 		return oxr_error(log, XR_ERROR_RUNTIME_FAILURE,
 		                 "Call to xsc->release_image failed");
 	}