[riot-devel] Byte array should be uint8_t, not char
Hi, The Coding Convention is clear about it. "Guidelines for pointer types (as long as it is reasonable): * use |char *| for strings and only for strings * use |uint8_t[]| as type for arbitrary byte buffers, but use |void *| to pass them around. |uint8_t[]| because we're dealing with bytes and not characters, |void *| to avoid unnecessary casting shall the need arise to have their content to have a certain type * use |uint8_t *| to pass "typed" byte buffers, e.g., link-layer addresses, where it avoids unnecessary temporary variable * use |void *| for generic typing" In the SPI driver however the transfer functions use char * parameters, but SPI is usually dealing with binary information (bytes), not strings. This leads to unnecessary casts in other parts of the code. (E.g. nvram_spi). What is our policy about this? Are we going to correct this at some point? Is it too late already (I hope not)? -- Kees Bakker Founder SODAQ M. 0031617737165 www.sodaq.com ___ devel mailing list devel@riot-os.org https://lists.riot-os.org/mailman/listinfo/devel
Re: [riot-devel] Byte array should be uint8_t, not char
Hi Kees, Unless there is a good reason to deviate from this guideline all violations should be corrected. This particular rule was added relatively recently, so it would not surprise me if not all occurrences in RIOT have been adapted yet. Cheers, Ludwig Am 3. Juli 2016 22:50:10 MESZ, schrieb Kees Bakker : >Hi, > >The Coding Convention is clear about it. > >"Guidelines for pointer types (as long as it is reasonable): > > * use |char *| for strings and only for strings > * use |uint8_t[]| as type for arbitrary byte buffers, but use |void *| >to pass them around. |uint8_t[]| because we're dealing with bytes >and not characters, |void *| to avoid unnecessary casting shall the >need arise to have their content to have a certain type > * use |uint8_t *| to pass "typed" byte buffers, e.g., link-layer >addresses, where it avoids unnecessary temporary variable > * use |void *| for generic typing" > > >In the SPI driver however the transfer functions use char * parameters, > >but SPI is usually dealing with binary >information (bytes), not strings. This leads to unnecessary casts in >other parts of the code. (E.g. nvram_spi). > >What is our policy about this? Are we going to correct this at some >point? Is it too late already (I hope not)? ___ devel mailing list devel@riot-os.org https://lists.riot-os.org/mailman/listinfo/devel
Re: [riot-devel] Progress of port to SODAQ Autonomo
Hi Kees, *, Am 2. Juli 2016 13:21:19 MESZ, schrieb Kees Bakker : >While going through the code I notice that there are too many "magic" >constants. Hard coded numbers that are obvious for some, but not >obvious >for others. My advise: always try to use defines and add a comment >about >what constants mean. Or point to datasheet sections explaining the >constants. Thank you for bringing this up. I am uncertain if there is something that can be done about it in the existing code base, but at least we should find a way to prevent such issues in the future: - Is there something in the (if, then probably Linux) coding conventions already? - if not, is anyone opposing the addition of such a rule? - if we have/want such a rule, can/should we add a static rule check for this in addition to the rule itself? I support adding such a rule of it does not exist yet. In my experience the false positives are heavily outweighed by true magic numbers of the kind we want to avoid. Therefore, for both the rule and the static rule check I would strongly recommend it. Cheers, Ludwig ___ devel mailing list devel@riot-os.org https://lists.riot-os.org/mailman/listinfo/devel
