Doing a logger in C

Question

In the following, I am trying to create a log utility in C:

#include <stdio.h>
#include <string.h>

enum LogLevel {DEBUG=10, INFO=20, WARN=30, ERROR=40};
typedef struct logger {
    int level;
    FILE** handlers; // NULL for sentinel
} Logger;
Logger LOGGER;

char* level_str(int level)
{
    switch (level) {
        case DEBUG: return "DEBUG";
        case INFO:  return "INFO";
        case WARN:  return "WARN";
        case ERROR:  return "ERROR";
        default: return "UNKNOWN";
    };
}
void logger(char* msg, int level)
{
    // skip if level is too low or no handlers set up
    if (level < LOGGER.level || !LOGGER.handlers) return;

    // write to each handler
    for (int i=0; LOGGER.handlers[i]; i++)
        fprintf(LOGGER.handlers[i], "[%s] %s\n", level_str(level), msg);
}
int main(void)
{
    LOGGER.level = DEBUG;
    FILE* handler1 = stderr;
    FILE* handler2 = fopen("z_log.txt", "w");
    LOGGER.handlers = &handler1;
    LOGGER.handlers[1] = handler2;
    LOGGER.handlers[2] = NULL;
    logger("Hello", INFO);
    return 0;
}

Working code here: https://onlinegdb.com/SJcoa5C7O.

My question is mainly about adding on some handler with these lines:

FILE* handler1 = stderr;
FILE* handler2 = fopen("z_log.txt", "w");
LOGGER.handlers = &handler1; // <-----------
LOGGER.handlers[1] = handler2;
LOGGER.handlers[2] = NULL;
logger("Hello", INFO);

I've noticed that the first handler needs a memory address, for example I can do:

LOGGER.handlers = &handler1;

But I cannot do:

LOGGER.handlers[0] = handler1;

Like I can with successive handlers that I set up. Why is this so? For example, why doesn't LOGGER.handlers[0] = handler1 set the memory address of LOGGER.handlers as I thought doing array[0] resolves to &array[0] so wouldn't that just be the initial memory address? Are there other ways to 'initialize' the pointer-to-pointers in the above?

It depends on what you want to do, but I think the double-pointer in the data type is probably a mistake. The `[0]` is potentially confusing because it's not an array; I would use `*` instead. — Neil, Mar 16 '21 at 21:02
@Neil ok -- the structure that I want though is an array of file handlers, so for example, I could log to multiple files (or, in the above, one handler going to `stderr` and one going to a log file). — carl.hiass, Mar 16 '21 at 21:03
One thing - `frprintf()` isn't guaranteed to perform write operations atomically, so if you have multiple processes logging to the same file, they'll write interleaved log entries. — Andrew Henle, Mar 16 '21 at 21:07
That's a valid use-case. One has to specify the array length, _eg_ `[5]`, use a VLA in C99, or `malloc`. — Neil, Mar 16 '21 at 21:07
Your main issue is that you need to allocate memory if you want to store multiple handlers. `handlers` can store only a single pointer only. — DaV, Mar 16 '21 at 21:08
@AndrewHenle I see, how is that usually done then to ensure atomicity (if that's the correct word.) — carl.hiass, Mar 16 '21 at 21:10
There aren't any easy ways around this. If you absolutely have to have it in order, lock the file before writing it, or ignore and just be aware. — Neil, Mar 16 '21 at 21:15
@Neil Sure there are. If you want to support multiple processes logging to the same file, you pretty much have to do something like `s[n]printf()` to create a single string and, on a POSIX system, use a single `write()` call to emit the string in a single operation. And you have to use `open()` with `O_APPEND` to ensure every write is atomically appended to the file. The atomic append guarantee might fail on network file systems like NFS, though. You can go even farther and implement macro logging calls that automatically capture function name, line number, and file. — Andrew Henle, Mar 16 '21 at 21:18
(cont) There are equivalent approaches on Windows, too. But I'm not familiar with those off hand. The nice thing with the POSIX calls is that you don't have to lock the file at all. See https://stackoverflow.com/questions/1154446/is-file-append-atomic-in-unix — Andrew Henle, Mar 16 '21 at 21:20
@Neil Not by much - if you're doing a lot of log entries, they're getting appended to the same pages in the page cache before getting flushed to disk. And if you're not doing a lot of log entries, holding onto the entry in the process's buffer just increases the chances the entry gets written out-of-order with respect to other processes. Nevermind the fact that `FILE *`-based buffering offers no controls to guarantee how the actual system calls that do the write are made. — Andrew Henle, Mar 16 '21 at 21:38

David Ranieri · Accepted Answer · 2021-03-16T21:16:55.743

5

You need to reserve space for the pointer to FILEs:

LOGGER.handlers = malloc(sizeof(*LOGGER.handlers) * 3);

then:

LOGGER.handlers[0] = handler1;
LOGGER.handlers[1] = handler2;
LOGGER.handlers[2] = NULL;

Or define FILE *handlers[3]; instead of FILE** handlers; if you know the number of elements beforehand.

edited Mar 16 '21 at 21:16

answered Mar 16 '21 at 21:08

David Ranieri

39,972
7
52
94

I see, thanks for that. Is using the `NULL` sentinel common for cases like this, or would the `LOGGER` structure store a size, or what's the suggested way to do that? – carl.hiass Mar 16 '21 at 21:11
3

It depends, your approach is valid (passing a `NULL` as sentinel), but if you kow the max number of elements then is better to use static memory, `FILE *handlers[MAX_HANDLERS];` and pass the number of elements. – David Ranieri Mar 16 '21 at 21:14
1

Unless there's a natural null-termination, I would probably have `size_t number_of_handlers` and forget the NULL, but maybe it's convenient for your use-case? – Neil Mar 16 '21 at 21:24
1

@DavidRanieri Unless the number of handlers are truly huge, a fixed one should do even for unknown. Each program will only have one instance of LOGGER. – klutt Mar 16 '21 at 21:33
@DavidRanieri when you say "static" do you mean defining it at file scope? Or just doing it at the top of `main`, or is it trivial which way to do it? – carl.hiass Mar 16 '21 at 21:34
1

@carl.hiass since you need only one structure for the program, you can define it directly in the `struct` : `#define NHANDLERS SOME_VALUE; typedef struct logger { int level; FILE *handlers[NHANDLERS]; } Logger;`, then, in your function: `for (size_t i = 0; i < NHANDLERS; i++)`, as pointed out by klutt, in this case you don't need a sentinel and `NHANDLERS` can be 2. – David Ranieri Mar 16 '21 at 21:40

Doing a logger in C

1 Answers1