[riot-notifications] [RIOT-OS/RIOT] WIP net/ndp: make addr array ptr c++ compliant (#11299)

Martine Lenders notifications at github.com
Thu Mar 28 09:20:59 CET 2019


miri64 requested changes on this pull request.

I'm fine with the change itself, just some comments regarding personal preference and style.

> @@ -238,8 +238,8 @@ gnrc_pktsnip_t *gnrc_ndp_opt_rdnss_build(uint32_t ltime, ipv6_addr_t *addrs,
         rdnss_opt->resv.u16 = 0;
         rdnss_opt->ltime = byteorder_htonl(ltime);
         for (unsigned i = 0; i < addrs_num; i++) {
-            memcpy(&rdnss_opt->addrs[i], &addrs[i],
-                   sizeof(rdnss_opt->addrs[i]));
+            memcpy(&(ndp_opt_rdnss_addrs_ptr(rdnss_opt)[i]), &addrs[i],

Can you rather use a temporary variable to appoint the result of `ndp_opt_rdnss_addrs_ptr(rdnss_opt)`, please? I think that would make the code a bit more readable.

>  } ndp_opt_rdnss_t;
 
+/**
+ * @brief   Get the start of addresses array in the recursive DNS server option
+ *
+ * @param[in]   opt   Recursive DNS server option in contiguous memory
+ *
+ * @returns     pointer to first address
+ */
+static inline ipv6_addr_t *ndp_opt_rdnss_addrs_ptr(ndp_opt_rdnss_t *opt) {

Non-compulsory name proposal: How about just `ndp_opt_rdnss_addrs()`? This would make the lines of usage a bit shorter.

>  } ndp_opt_rdnss_t;
 
+/**
+ * @brief   Get the start of addresses array in the recursive DNS server option
+ *
+ * @param[in]   opt   Recursive DNS server option in contiguous memory
+ *
+ * @returns     pointer to first address
+ */
+static inline ipv6_addr_t *ndp_opt_rdnss_addrs_ptr(ndp_opt_rdnss_t *opt) {
+	return (ipv6_addr_t *) (((uint8_t *)opt) + sizeof(ndp_opt_rdnss_t));

Not MISRA-compliant, but I prefer the 

```suggestion
	return (ipv6_addr_t *) (opt + 1);
```

As it is way more readable in my opinion (but I know pointer arithmetics, so I can accept that people who don't might find this confusing).

-- 
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/11299#pullrequestreview-219877797
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20190328/57b1c805/attachment.html>


More information about the notifications mailing list