[riot-notifications] [RIOT-OS/RIOT] sys: add unaligned.h (header for alignment safe value-from-pointer functions) (#10797)

Martine Lenders notifications at github.com
Thu Jan 24 12:37:00 CET 2019


miri64 commented on this pull request.

Some style and doc comments before I give my final ACK.

> +extern "C" {
+#endif
+
+/** @brief Unaligned access helper struct (uint16_t version) */
+typedef struct {
+    uint16_t val;       /**< value */
+} __attribute__((packed)) uint16_una_t;
+
+/**
+ * @brief    Get uint16_t from possibly unaligned pointer
+ *
+ * @param[in]   ptr pointer to read from
+ *
+ * @returns value read from @p ptr
+ */
+static inline uint16_t get_unaligned_u16(const void *ptr)

According to naming conventions this should be `unaligned_get_u16()` or something.

> + */
+
+/**
+ * @defgroup    sys_unaligned unaligned access methods
+ * @ingroup     sys
+ * @brief       Provides functions for safe unaligned memory accesses
+ *
+ * This header provides functions to read values from pointers that are not
+ * necessarily aligned to the type's alignment requirements.
+ *
+ * E.g.,
+ *
+ *     uint16_t *foo = 0x123;
+ *     printf("%u\n", *foo);
+ *
+ * ... might cause an unaligned access, if uint16_t's is usually aligned ad

rather prefer

```suggestion
 * ... might cause an unaligned access, if `uint16_t`'s is usually aligned ad
```

here.

> + */
+
+/**
+ * @defgroup    sys_unaligned unaligned access methods
+ * @ingroup     sys
+ * @brief       Provides functions for safe unaligned memory accesses
+ *
+ * This header provides functions to read values from pointers that are not
+ * necessarily aligned to the type's alignment requirements.
+ *
+ * E.g.,
+ *
+ *     uint16_t *foo = 0x123;
+ *     printf("%u\n", *foo);
+ *
+ * ... might cause an unaligned access, if uint16_t's is usually aligned ad

Also `s/ad$/at/` ;-)

> @@ -0,0 +1,69 @@
+/*
+ * Copyright (C) 2019 Kaspar Schleiser <kaspar at schleiser.de>
+ *
+ * This file is subject to the terms and conditions of the GNU Lesser
+ * General Public License v2.1. See the file LICENSE in the top level
+ * directory for more details.
+ */
+
+/**
+ * @defgroup    sys_unaligned unaligned access methods

Since this shows up in top-level doc, I prefer

```suggestion
 * @defgroup    sys_unaligned unaligned memory access methods
```

> + * @author      Kaspar Schleiser <kaspar at schleiser.de>
+ */
+
+#ifndef UNALIGNED_H
+#define UNALIGNED_H
+
+#include <stdint.h>
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/** @brief Unaligned access helper struct (uint16_t version) */
+typedef struct {
+    uint16_t val;       /**< value */
+} __attribute__((packed)) uint16_una_t;

Most of our own code puts the attribute directly after `struct`:

```
$ git grep "__attribute__((packed))" | wc -l
97
$ git grep "\(struct\|union\) __attribute__((packed))" | wc -l
91
$ git grep "__attribute__((packed))" | grep -v "\(struct\|union\) __attribute__((packed))"
cpu/esp32/vendor/esp-idf/include/esp32/esp_mesh.h:} __attribute__((packed)) mip_t;
cpu/esp32/vendor/esp-idf/include/esp32/esp_mesh.h:} __attribute__((packed)) mesh_opt_t;
cpu/esp32/vendor/esp-idf/include/esp32/esp_mesh_internal.h:} __attribute__((packed)) mesh_assoc_t;
cpu/esp8266/vendor/espressif/c_types.h:/* #define __packed        __attribute__((packed)) */
pkg/lwip/include/arch/cc.h:#define PACK_STRUCT_STRUCT      __attribute__((packed))
sys/include/net/gnrc/pktbuf.h: *          (defined with `__attribute__((packed))`) or enforce alignment in
```

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/RIOT-OS/RIOT/pull/10797#pullrequestreview-195980004
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20190124/3fbc1053/attachment.html>


More information about the notifications mailing list