Skip to content
Permalink
Browse files

Revert "sys/util.h: helper macro to perform pointer difference"

This reverts commit 755cc64.

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 <pab@pabigot.com>
  • Loading branch information...
pabigot authored and nashif committed Jul 13, 2019
1 parent caa47e6 commit 765c06376c12ad332d792fa19a6f4b40d2841ad0
Showing with 13 additions and 11 deletions.
  1. +0 −3 include/sys/util.h
  2. +13 −8 tests/subsys/usb/desc_sections/src/desc_sections.c
@@ -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
@@ -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);

0 comments on commit 765c063

Please sign in to comment.
You can’t perform that action at this time.