log.h
I don't think that calling code has any need for TS_BUF_LENGHT, so this could be moved to the implementation file. Consider spelling LENGTH the usual way, too.
FILE and uint8_t are undefined. You need to include <stdio.h> and `<stdint.h>, as it's not possible to portably forward-declare these typedefs.
Note that uint8_t is provided only when the underlying hardware has support for 8-bit values without padding. There's no good reason to restrict the number of flags so tightly this early in development - I recommend using plain unsigned int there.
log.c
We're missing declarations from <time.h> and <stdio.h>.
Don't ignore the return value: if we're running in a locale where the output exceeds the provided maximum length, then
strftime()returns 0, and the contents of the array are undefined.
(my emphasis). So check the result before using the buffer. We can reduce the work in this function by calling the time-related functions only when we're actually going to use the output:
void log_msg (FILE * fp, const char *msg, unsigned options)
{
time_t time_val = options & LOG_FULLTIME ? time (0) : 0;
static long long log_count = 0;
char time_stamp[TS_BUF_LENGTH];
char date_stamp[TS_BUF_LENGTH];
struct tm *tm_info = options & LOG_FULLTIME ? localtime (&time_val) : NULL;
if (options & LOG_COUNT) {
fprintf (fp, "%lld\n, ", ++log_count);
}
if (options & LOG_DATE && strftime (date_stamp, TS_BUF_LENGTH, "%F (%a)", tm_info)) {
fprintf (fp, "%s, ", date_stamp);
}
if (options & LOG_TIME && strftime (time_stamp, TS_BUF_LENGTH, "%H:%M:%S", tm_info)) {
fprintf (fp, "%s, ", time_stamp);
}
fprintf (fp, "%s\n", msg);
}
If this program is to run as a daemon, it's probably better to allow use of syslog() - that takes care of re-opening the file during log rotation, for example.
selectserver.h
That's a lot of includes for a header file! None of them seem to be necessary (though it's polite to include <stdio.h> so the macros can be expanded).
However, I don't think we need this header at all - the implementation is where main() is, so these definitions can simply all go into selectserver.c.
POSIX_C_SOURCE should expand to a long constant, so it's best to define it as 200809L.
ARRAY_CARDINALITY() improperly protects its argument from precedence errors - we need (x)[0] rather than (x[0]):
#define ARRAY_CARDINALITY(x) (sizeof (x) / sizeof (x)[0])
The FFLUSH() and FSYNC() macros are dangerous - use the do/while(0) idiom to protect against matching the wrong else:
#define FFLUSH(fp) do { if (fflush (fp) == EOF) perror ("fflush()"); } while (0)
#define FSYNC(fd) do { if (fsync (fd) == -1) perror ("fsync()"); } while (0)
selectserver.c
There's a lot of unnecessary resetting of errno = 0: in most cases, we test function return values and only access errno if the function has indicated that it has written to it. We can trust that these functions behave as advertised, and eliminate those effectively-dead writes.
The signal handlers appear to be overkill, since we flush logs each time we write them anyway (and I think that responsibility ought to move to log.c, rather than mixed into the application code).
In init_addr() we only ever assign to status and never use it. So that variable can be eliminated.
In init_sock() we should close master_fd before we continue when setsockopt() fails. Actually, we shouldn't continue to the next iteration - just log the failure and attempt to bind regardless.
Similarly, when we fail to listen(), we also leak the file descriptor.
accept_new_connection() also leaks fds - whenever we return something other than slave_fd, we need to ensure that it is closed first. Again, consider proceeding despite failure to set options.
In read_line, we have if (!buf) { … free(buf); }. That's not wrong, but is redundant. And the subsequent status = 0 is pointless, as that's immediately followed by return (or perhaps *status = 0 was intended - it's not clear what the status values mean).
We could eliminate the duplication of the realloc() calls an consequent tests by starting with buf = NULL and moving the allocation to the beginning of the loop.
I think this is wrong:
char *new = realloc (buf, page_size + page_size);
Perhaps total + page_size?
The redefinition of ARRAY_CARDINALITY() suffers the same problem as the first one.
Makefile
A lot of the warnings are enabled by -Wall -Wextra, so are redundant. I normally write them on separate lines, like this:
CFLAGS += -std=c17
CFLAGS += -O2
CFLAGS += -pedantic-errors
CFLAGS += -Wall
CFLAGS += -Wextra
⋮
That makes it easier to temporarily change the flags used by commenting out lines.
We don't need a %.o: %.c rule, as Make already knows about that, and if we just call the executable selectserver then Make can infer that too.
log.o shouldn't depend on selectserver.h - instead of writing dependencies yourself, just ask GCC to do it as a side-effect of compilation, like this:
CFLAGS += -MD
-include $(wildcard *.d)
We're missing .PHONY and .DELETE_ON_ERROR targets, which makefiles always need.
A replacement makefile is a bit simpler:
# the compiler:
CC = gcc-10
# compiler flags:
CFLAGS += -std=c17
CFLAGS += -Wall
CFLAGS += -Wextra
CFLAGS += -Warray-bounds
CFLAGS += -Wconversion
CFLAGS += -Wmissing-braces
CFLAGS += -Wno-parentheses
CFLAGS += -Wpedantic
CFLAGS += -Wstrict-prototypes
CFLAGS += -Wwrite-strings
CFLAGS += -fanalyzer
CFLAGS += -fno-builtin
CFLAGS += -fno-common
CFLAGS += -fno-omit-frame-pointer
CFLAGS += -fsanitize=address
CFLAGS += -fsanitize=undefined
CFLAGS += -O2
CFLAGS += -MD
selectserver: selectserver.o log.o
clean:
$(RM) *.o *.d selectserver
-include $(wildcard *.d)
.PHONY: clean
.DELETE_ON_ERROR: