From 765c06376c12ad332d792fa19a6f4b40d2841ad0 Mon Sep 17 00:00:00 2001 From: "Peter A. Bigot" Date: Fri, 12 Jul 2019 22:04:15 -0500 Subject: [PATCH] Revert "sys/util.h: helper macro to perform pointer difference" This reverts commit 755cc644cc0b47dfa335219cc305a04a14e4ec25. This approach is problematic in several ways. First, `intptr_t` could cause undefined behavior in the subtraction when the pointer converts to a negative value. Except in weird cases where the sign of the pointer identifies a memory domain (like kernel vs userspace) I'm unaware of any valid use of `intptr_t`. Second, this macro was created to address a special need that cannot rely on defined behavior: i.e. to ensure that data definitions are placed in contiguous space and access is provided through linker-defined symbols, for which the language required alignment and continuity is not guaranteed. A macro that calculates the span between linker symbols has very different semantics than one that calculates the difference between pointers. Replace the global PTR_DIFF with a documented local macro that tests what's necessary without risking integer overflow. Signed-off-by: Peter A. Bigot --- include/sys/util.h | 3 --- .../usb/desc_sections/src/desc_sections.c | 21 ++++++++++++------- 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/include/sys/util.h b/include/sys/util.h index b678dec08bc..c3655705759 100644 --- a/include/sys/util.h +++ b/include/sys/util.h @@ -26,9 +26,6 @@ #define POINTER_TO_INT(x) ((intptr_t) (x)) #define INT_TO_POINTER(x) ((void *) (intptr_t) (x)) -/* Helper to get the difference between two pointers as an int */ -#define PTR_DIFF(y, x) ((int)((intptr_t)(y) - (intptr_t)(x))) - #if !(defined (__CHAR_BIT__) && defined (__SIZEOF_LONG__)) # error Missing required predefined macros for BITS_PER_LONG calculation #endif diff --git a/tests/subsys/usb/desc_sections/src/desc_sections.c b/tests/subsys/usb/desc_sections/src/desc_sections.c index 2d3b8377b13..5b13a451fac 100644 --- a/tests/subsys/usb/desc_sections/src/desc_sections.c +++ b/tests/subsys/usb/desc_sections/src/desc_sections.c @@ -198,41 +198,46 @@ static void check_endpoint_allocation(struct usb_desc_header *head) } } +/* Determine the number of bytes spanned between two linker-defined + * symbols that are normally interpreted as pointers. + */ +#define SYMBOL_SPAN(_ep, _sp) (int)(intptr_t)((uintptr_t)(_ep) - (uintptr_t)(_sp)) + static void test_desc_sections(void) { struct usb_desc_header *head; TC_PRINT("__usb_descriptor_start %p\n", __usb_descriptor_start); TC_PRINT("__usb_descriptor_end %p\n", __usb_descriptor_end); - TC_PRINT("USB Descriptor table size %d\n", - PTR_DIFF(__usb_descriptor_end, __usb_descriptor_start)); + TC_PRINT("USB Descriptor table span %d\n", + SYMBOL_SPAN(__usb_descriptor_end, __usb_descriptor_start)); TC_PRINT("__usb_data_start %p\n", __usb_data_start); TC_PRINT("__usb_data_end %p\n", __usb_data_end); - TC_PRINT("USB Configuration data size %d\n", - PTR_DIFF(__usb_data_end, __usb_data_start)); + TC_PRINT("USB Configuration data span %d\n", + SYMBOL_SPAN(__usb_data_end, __usb_data_start)); TC_PRINT("sizeof usb_cfg_data %zu\n", sizeof(struct usb_cfg_data)); LOG_DBG("Starting logs"); LOG_HEXDUMP_DBG((u8_t *)__usb_descriptor_start, - PTR_DIFF(__usb_descriptor_end, __usb_descriptor_start), + SYMBOL_SPAN(__usb_descriptor_end, __usb_descriptor_start), "USB Descriptor table section"); LOG_HEXDUMP_DBG((u8_t *)__usb_data_start, - PTR_DIFF(__usb_data_end, __usb_data_start), + SYMBOL_SPAN(__usb_data_end, __usb_data_start), "USB Configuratio structures section"); head = (struct usb_desc_header *)__usb_descriptor_start; zassert_not_null(head, NULL); - zassert_equal(PTR_DIFF(__usb_descriptor_end, __usb_descriptor_start), + zassert_equal(SYMBOL_SPAN(__usb_descriptor_end, __usb_descriptor_start), 133, NULL); /* Calculate number of structures */ zassert_equal(__usb_data_end - __usb_data_start, NUM_INSTANCES, NULL); - zassert_equal(PTR_DIFF(__usb_data_end, __usb_data_start), + zassert_equal(SYMBOL_SPAN(__usb_data_end, __usb_data_start), NUM_INSTANCES * sizeof(struct usb_cfg_data), NULL); check_endpoint_allocation(head);