Fri 16 Jan 2015 05:13:32 PM UTC, comment #1:
My proposal is to add simple helper functions for common string operations. See below for details.
Patches from 0001-* to 0006-* introduce new string helpers (as macros):
- str_starts_with(str, prefix) - checks if str starts with the specified prefix,
- str_appendf(str, alloc_size, format, ...) - appends formatted output to specified string (with allocated size check),
- str_vappendf(str, alloc_size, format, ...) - va_list variant of str_appendf,
- str_rstrip_char(str, ch) - strips the last character from str if it's equal to ch
In order to verify, whether these changes don't break anything, just build the code before and after applying the patches, with:
- gpsd_version set to string without “~dev” suffix,
- “-DNDEBUG” in CFLAGS.
In both cases stripped binaries should be identical.
Patch 0007-* converts all macros to functions. The conversion is safe, as long as each argument used in a macro more than once is free of side effects. In order to verify this, I temporarily changed definitions of the macros, adding extra occurrence of each argument, which was used more than once (patch “strfuncs-verify-side-effects.diff”). The change didn't affect the generated binary (with settings like above), which means that there were no side effects in these arguments.
Patches 0008-* to 0010-* do change the generated code, but they are rather small. The code passes regression tests after each patch.
|
Fri 16 Jan 2015 05:09:14 PM UTC, original submission:
Gpsd contains a lot of boilerplate code for commonly performed string operations, such as:
- appending formatted string: (void)snprintf(msgbuf + strlen(msgbuf), sizeof(msgbuf) - strlen(msgbuf), "...", ...);
- stripping single character at the end: if (reply[strlen(reply) - 1] == ',') reply[strlen(reply) - 1] = '\0';
- checking whether the string begins with specified prefix: strncmp(session->gpsdata.dev.path, "blahblah", 8) != 0
The problem with boilerplate code is that:
- it's hard to read,
- it's error-prone (see bug #43953 for examples, I'll report more such problems with fixes soon).
|