On Sun, May 14, 2023 at 09:55:16PM +0200, Sebastiano Tronto wrote:
> I could not find any tool that was simple enough for my taste, so I
> rolled my own[0].
>
> [0] https://git.tronto.net/sdep
Not too bad, and since the source was pretty small, I decided to take a
glance. Here's some unsolicited review:
9 #define min(X,Y) (X<Y ? X : Y)
Macros like these can produce unintended results due to operator precedence.
Perhaps it's not an issue in your usage code, but it's best to avoid having
macros with such pitfalls to begin with.
https://c-faq.com/cpp/safemacros.html
11 /*
12 * Maximum number of characters in a line. The rest will be
truncated.
13 * Change this if you need very long lines.
14 */
15 static const int MAXLEN = 10000;
`const` in C doesn't mean "constant expression", and so if you use
`buf[MAXLEN]` you'll get a VLA (compile with -Wvla and you should see warning
about it). Either use a #define or an enum.
https://c-faq.com/ansi/constasconst.html
55 static Options default_op();
`f()` does not do what you might think. A function without an argument list
takes *unspeficied* amount of arguments, not zero arguments (a historical
baggage). These have been obsolete since C99 and newer clang version defaults
to erroring out on it, see: https://wiki.gentoo.org/wiki/Modern_C_porting
Explicitly use `f(void)` instead.
71 next->ev.text = malloc(sizeof(char) * l);
sizeof(char) is *defined* to be always 1. So it's not really doing anything:
https://c-faq.com/malloc/sizeofchar.html
72 strncpy(next->ev.text, text, l);
`strncpy` doesn't nul-terminate in case the soruce is bigger than the dest.
Additionally strncpy will *always* write `n` bytes even if the soruce fits into
the dest. This is rarely the semantic people want and 99% of the time I see
strncpy used it's typically either bugger, misused, or enefficient.
There is no standard "copy and truncate if needed" function. Closest you can
find would be memccpy:
if (memccpy(dest, src, '\0', n) == NULL)
dest[n - 1] = '\0';
You can wrap this in function or roll your own (TIP: if you have your string
copy
function return a pointer to the nul-byte in dest, then you can use it for
efficient concat: https://www.symas.com/post/the-sad-state-of-c-strings).
216 static char *
217 strtrim(char *t)
218 {
219 char *s;
220
221 for (s = &t[strlen(t)-1]; s != t && isspace(*s); *s = '\0',
s--);
222 for (; *t != '\0' && isspace(*t); t++);
The entire ctype library is badly designed because it only accepts input that's
either EOF or [0, UCHAR_MAX]. From the manpage:
| These functions check whether c, which must have the value of an unsigned char
| or EOF (otherwise the behavior is undefined)
Either cast the argument to `unsigned char` or just roll your own. (Also
keep in mind that `plain char` can be either signed or unsigned
depending on the implementation).
Also the name `strtrim` steps into the namespace reserved by <string.h>.
You can rename it to `str_trim` to avoid it.
And that's mostly it from a quick glance. Slightly relevant:
http://www.catb.org/~esr/writings/unix-koans/ten-thousand.html
- NRK
