Дана 24/09/25 08:21PM, bi...@iscarioth.org написа: > Sorry for the inconvenience, I wanted to know if this > piece of code seemed to be correct for a OpenBSD developer ?
Dear Bilal, I think that C code review is out of the scope of this list. You could try at: https://codereview.stackexchange.com/ For kernel developer style guide, see style(9). My proverbial two cents: > #define SAFE_FREE(addr) do { \ > if (addr) \ > { \ > free (addr); \ > addr = NULL; \ > } \ > else \ > { \ > fprintf (stderr, " (double free inside %s:%s) at L%d\n", \ > __FILE__, __func__, __LINE__); \ > } \ > } while(0); - The do { } while(0) idiom inside macros should not end with a semicolon. The idea for the usage of such macros is to write something like SAFE_FREE(addr); which if written like in your email would be expanded to do { // etc... } while(0);; - You don't need to check if addr is NULL. See the description of free in free(3). Setting addr to NULL is sufficient to prevent "double free". Of course, also keep in mind variable scope. > const size_t len = strlen (*s); This depends on the context, but consider what would happen if *s contains a sequence of chars which is not NUL-terminated (not a string): disaster. > char *new_line = realloc (line, len + offset + 2); > if (new_line == NULL) > { > if (line != NULL) > { > SAFE_FREE (line); > } > break; > } Typically, if (re)allocation fails, you want to abort the program as soon as possible. So something like this would be preferable: char* new_line = realloc(line, len + offset + 2); if (!new_line) { perror("realloc"); exit(errno); } What's more, in a recent discussion the case of failing memory (re)allocation was explicitly mentioned as the one where freeing memory is detrimental. > line = new_line; > memset (&line[offset], 0, len); > strcat (strcat (&line[offset], *s), " "); > offset += len + 1 ; You should check out memccpy(3), very roughly said: if properly used it is much safer than the alternatives, and doubles as strcat.
