On 12.03.2022 22:21:42, Max Staudt wrote:
> @Vincent - two more things have remained, and I hope it's okay once I
> explain them:
>
> 1. _memstrcmp() - memcmp() vs. str(n)cmp()
>
> The _memstrcmp() function does not compare strings, it compares raw
> buffers. I am just using C strings for the fixed buffers to compare
> against, as that allows for shorter and easier to read code. The NUL
> byte at the end of those strings goes unused.
>
> Also, I have not looked at the assembly produced, since the semantics
> are different: str(n)cmp() needs to look for NUL bytes in the buffer(s),
> which is unnecessary here. As a bonus, NUL will never even occur
> because my code filters those bytes out upon reception from the UART
> (it's a documented quirk of the ELM327).
>
> Finally, even if I were to use strcmp(), the code would still look just
> as ugly. Except the machine would also look for NUL bytes, and the next
> human to read the code would wonder why I'm comparing strings and not
> buffers.
>
> Hence memcmp(), to help the code self-document and the compiler
> optimise - I hope that's okay.
Looking at the code:
> +/* Compare a buffer to a fixed string */
> +static inline int _memstrcmp(const u8 *mem, const char *str)
> +{
> + return memcmp(mem, str, strlen(str));
The _memstrcmp is sometimes directly used. Where's the check that mem is
valid for strlen(len)? Better only use _len_memstrcmp().
> +}
> +
> +/* Compare buffer to string length, then compare buffer to fixed string.
> + * This ensures two things:
> + * - It flags cases where the fixed string is only the start of the
> + * buffer, rather than exactly all of it.
> + * - It avoids byte comparisons in case the length doesn't match.
> + */
> +static inline int _len_memstrcmp(const u8 *mem, size_t mem_len, const char *str)
make it a bool.
> +{
> + size_t str_len = strlen(str);
> +
> + return (mem_len != str_len) || memcmp(mem, str, str_len);
> +}
Can you rename _len_memstrcmp into something like elm327_match() or so?
regards,
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Embedded Linux | https://www.pengutronix.de |
Vertretung West/Dortmund | Phone: +49-231-2826-924 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
Attachment:
signature.asc
Description: PGP signature
