From f65a77adf806025f84dce23880a518ca1bd70f61 Mon Sep 17 00:00:00 2001 From: David Robillard Date: Mon, 26 Nov 2018 22:14:39 +0100 Subject: Add extensible logging API --- NEWS | 1 + serd/serd.h | 121 +++++++++++++++++++++++++++++++++++++++---------- src/reader.c | 8 +++- src/reader.h | 2 +- src/serdi.c | 7 +-- src/world.c | 146 +++++++++++++++++++++++++++++++++++++++++++++++++---------- src/world.h | 32 +++++++++++-- src/writer.c | 20 ++++---- 8 files changed, 271 insertions(+), 66 deletions(-) diff --git a/NEWS b/NEWS index 831c6c7a..0dd10be1 100644 --- a/NEWS +++ b/NEWS @@ -2,6 +2,7 @@ serd (1.0.1) unstable; * Add SerdBuffer for mutable buffers to keep SerdChunk const-correct * Add SerdWorld for shared library state + * Add extensible logging API * Bring read/write interface closer to C standard * Make nodes opaque * Make serd_strtod API const-correct diff --git a/serd/serd.h b/serd/serd.h index 9c20dc0f..153d7113 100644 --- a/serd/serd.h +++ b/serd/serd.h @@ -42,6 +42,12 @@ # define SERD_API #endif +#if defined(__GNUC__) +# define SERD_LOG_FUNC(fmt, arg1) __attribute__((format(printf, fmt, arg1))) +#else +# define SERD_LOG_FUNC(fmt, arg1) +#endif + #ifdef __cplusplus extern "C" { #endif @@ -192,14 +198,6 @@ typedef struct { size_t len; ///< Size of buffer in bytes } SerdBuffer; -/// An error description -typedef struct { - SerdStatus status; /**< Error code */ - const SerdCursor* cursor; /**< Origin of error, or NULL */ - const char* fmt; /**< Message format string (printf style) */ - va_list* args; /**< Arguments for fmt */ -} SerdError; - /** A parsed URI @@ -756,15 +754,6 @@ serd_node_free(SerdNode* node); @{ */ -/** - Sink (callback) for errors. - - @param handle Handle for user data. - @param error Error description. -*/ -typedef SerdStatus (*SerdErrorSink)(void* handle, - const SerdError* error); - /** Sink (callback) for base URI changes @@ -842,16 +831,102 @@ const SerdNode* serd_world_get_blank(SerdWorld* world); /** - Set a function to be called when errors occur. + @} + @name Logging + @{ +*/ + +/// Log message level, compatible with syslog +typedef enum { + SERD_LOG_LEVEL_EMERG, ///< Emergency, system is unusable + SERD_LOG_LEVEL_ALERT, ///< Action must be taken immediately + SERD_LOG_LEVEL_CRIT, ///< Critical condition + SERD_LOG_LEVEL_ERR, ///< Error + SERD_LOG_LEVEL_WARNING, ///< Warning + SERD_LOG_LEVEL_NOTICE, ///< Normal but significant condition + SERD_LOG_LEVEL_INFO, ///< Informational message + SERD_LOG_LEVEL_DEBUG ///< Debug message +} SerdLogLevel; + +/** + A structured log field. + + This can be used to pass additional information along with log messages. + Syslog-compatible keys should be used where possible, otherwise, keys should + be namespaced to prevent clashes. + + Serd itself uses the following keys: + - ERRNO + - SERD_COL + - SERD_FILE + - SERD_LINE + - SERD_STATUS +*/ +typedef struct { + const char* key; ///< Field name + const char* value; ///< Field value +} SerdLogField; + +/** + A log entry (message). + + This is the description of a log entry which is passed to log functions. + It is only valid in the stack frame it appears in, and may not be copied. +*/ +typedef struct { + const char* domain; ///< Message domain (library or program name) + SerdLogLevel level; ///< Log level + unsigned n_fields; ///< Number of entries in `fields` + const SerdLogField* fields; ///< Extra log fields + const char* fmt; ///< Format string (printf style) + va_list* args; ///< Arguments corresponding to fmt +} SerdLogEntry; + +/** + Sink function for log messages. + + @param handle Handle for user data. + @param entry Pointer to log entry description. +*/ +typedef SerdStatus (*SerdLogFunc)(void* handle, const SerdLogEntry* entry); + +/// Return the value of the log field named `key`, or NULL if none exists +SERD_API const char* +serd_log_entry_get_field(const SerdLogEntry* entry, const char* key); + +/** + Set a function to be called with log messages (typically errors). - The `error_sink` will be called with `handle` as its first argument. If - no error function is set, errors are printed to stderr. + The `log_func` will be called with `handle` as its first argument. If + no function is set, messages are printed to stderr. */ SERD_API void -serd_world_set_error_sink(SerdWorld* world, - SerdErrorSink error_sink, - void* handle); +serd_world_set_log_func(SerdWorld* world, SerdLogFunc log_func, void* handle); + +/// Write a message to the log +SERD_API +SERD_LOG_FUNC(6, 0) +SerdStatus +serd_world_vlogf(const SerdWorld* world, + const char* domain, + SerdLogLevel level, + unsigned n_fields, + const SerdLogField* fields, + const char* fmt, + va_list args); + +/// Write a message to the log +SERD_API +SERD_LOG_FUNC(6, 7) +SerdStatus +serd_world_logf(const SerdWorld* world, + const char* domain, + SerdLogLevel level, + unsigned n_fields, + const SerdLogField* fields, + const char* fmt, + ...); /** @} diff --git a/src/reader.c b/src/reader.c index 44117a20..248eaff7 100644 --- a/src/reader.c +++ b/src/reader.c @@ -39,8 +39,12 @@ r_err(SerdReader* reader, SerdStatus st, const char* fmt, ...) { va_list args; va_start(args, fmt); - const SerdError e = { st, &reader->source.cur, fmt, &args }; - serd_world_error(reader->world, &e); + serd_world_vlogf_internal(reader->world, + st, + SERD_LOG_LEVEL_ERR, + &reader->source.cur, + fmt, + args); va_end(args); return st; } diff --git a/src/reader.h b/src/reader.h index 3d59bf9c..302c8a7b 100644 --- a/src/reader.h +++ b/src/reader.h @@ -52,7 +52,7 @@ typedef struct { struct SerdReaderImpl { SerdWorld* world; const SerdSink* sink; - SerdErrorSink error_sink; + SerdLogFunc log_func; void* error_handle; SerdNode* rdf_first; SerdNode* rdf_rest; diff --git a/src/serdi.c b/src/serdi.c index c2ac567c..a8301c49 100644 --- a/src/serdi.c +++ b/src/serdi.c @@ -25,6 +25,7 @@ #endif #include +#include #include #include #include @@ -76,10 +77,10 @@ missing_arg(const char* name, char opt) } static SerdStatus -quiet_error_sink(void* handle, const SerdError* e) +quiet_error_func(void* handle, const SerdLogEntry* entry) { (void)handle; - (void)e; + (void)entry; return SERD_SUCCESS; } @@ -219,7 +220,7 @@ main(int argc, char** argv) serd_reader_set_strict(reader, !lax); if (quiet) { - serd_world_set_error_sink(world, quiet_error_sink, NULL); + serd_world_set_log_func(world, quiet_error_func, NULL); } SerdNode* root = serd_new_uri(root_uri); diff --git a/src/world.c b/src/world.c index f950e948..1c090f8d 100644 --- a/src/world.c +++ b/src/world.c @@ -34,14 +34,21 @@ #define BLANK_CHARS 11 +static const char* const log_level_strings[] = { "emergengy", "alert", + "critical", "error", + "warning", "note", + "info", "debug" }; + FILE* serd_world_fopen(SerdWorld* world, const char* path, const char* mode) { FILE* fd = fopen(path, mode); if (!fd) { - serd_world_errorf(world, SERD_ERR_INTERNAL, - "failed to open file %s (%s)\n", - path, strerror(errno)); + char errno_str[24]; + snprintf(errno_str, sizeof(errno_str), "%d", errno); + const SerdLogField fields[] = { { "ERRNO", errno_str } }; + serd_world_logf(world, "serd", SERD_LOG_LEVEL_ERR, 1, fields, + "failed to open file %s (%s)\n", path, strerror(errno)); return NULL; } #if defined(HAVE_POSIX_FADVISE) && defined(HAVE_FILENO) @@ -50,32 +57,127 @@ serd_world_fopen(SerdWorld* world, const char* path, const char* mode) return fd; } +static const char* +get_log_field(const SerdLogField* const fields, + const size_t n_fields, + const char* const key) +{ + for (size_t i = 0; i < n_fields; ++i) { + if (!strcmp(fields[i].key, key)) { + return fields[i].value; + } + } + + return NULL; +} + +const char* +serd_log_entry_get_field(const SerdLogEntry* const entry, const char* const key) +{ + return get_log_field(entry->fields, entry->n_fields, key); +} + SerdStatus -serd_world_error(const SerdWorld* world, const SerdError* e) +serd_world_vlogf(const SerdWorld* world, + const char* domain, + SerdLogLevel level, + const unsigned n_fields, + const SerdLogField* fields, + const char* fmt, + va_list args) { - if (world->error_sink) { - world->error_sink(world->error_handle, e); + // Copy args (which may be an array) to portably get a pointer + va_list ap; + va_copy(ap, args); + + const SerdLogEntry e = {domain, level, n_fields, fields, fmt, &ap}; + SerdStatus st = SERD_SUCCESS; + + if (world->log_func) { + st = world->log_func(world->log_handle, &e); } else { - fprintf(stderr, "error: "); - if (e->cursor) { - fprintf(stderr, - "%s:%u:%u: ", - serd_node_get_string(e->cursor->file), - e->cursor->line, - e->cursor->col); + // Print GCC-style level prefix (error, warning, etc) + fprintf(stderr, "%s: ", log_level_strings[level]); + + // Add input file and position to prefix if available + const char* const file = serd_log_entry_get_field(&e, "SERD_FILE"); + const char* const line = serd_log_entry_get_field(&e, "SERD_LINE"); + const char* const col = serd_log_entry_get_field(&e, "SERD_COL"); + if (file && line && col) { + fprintf(stderr, "%s:%s:%s: ", file, line, col); } - vfprintf(stderr, e->fmt, *e->args); + + // Using a copy isn't necessary here, but it avoids a clang-tidy bug + vfprintf(stderr, fmt, ap); } - return e->status; + + va_end(ap); + return st; +} + +SerdStatus +serd_world_logf(const SerdWorld* world, + const char* domain, + SerdLogLevel level, + const unsigned n_fields, + const SerdLogField* fields, + const char* fmt, + ...) +{ + va_list args; + va_start(args, fmt); + + const SerdStatus st = + serd_world_vlogf(world, domain, level, n_fields, fields, fmt, args); + + va_end(args); + return st; +} + +SerdStatus +serd_world_vlogf_internal(const SerdWorld* world, + SerdStatus st, + SerdLogLevel level, + const SerdCursor* cursor, + const char* fmt, + va_list args) +{ + char st_str[8]; + snprintf(st_str, sizeof(st_str), "%d", st); + if (cursor) { + const char* file = serd_node_get_string(serd_cursor_get_name(cursor)); + + char line[24]; + snprintf(line, sizeof(line), "%u", serd_cursor_get_line(cursor)); + + char col[24]; + snprintf(col, sizeof(col), "%u", serd_cursor_get_column(cursor)); + + const SerdLogField fields[] = { { "SERD_STATUS", st_str }, + { "SERD_FILE", file }, + { "SERD_LINE", line }, + { "SERD_COL", col } }; + + serd_world_vlogf(world, "serd", level, 4, fields, fmt, args); + } else { + const SerdLogField fields[] = { { "SERD_STATUS", st_str } }; + serd_world_vlogf(world, "serd", level, 1, fields, fmt, args); + } + + return st; } SerdStatus -serd_world_errorf(const SerdWorld* world, SerdStatus st, const char* fmt, ...) +serd_world_logf_internal(const SerdWorld* world, + SerdStatus st, + SerdLogLevel level, + const SerdCursor* cursor, + const char* fmt, + ...) { va_list args; va_start(args, fmt); - const SerdError e = { st, NULL, fmt, &args }; - serd_world_error(world, &e); + serd_world_vlogf_internal(world, st, level, cursor, fmt, args); va_end(args); return st; } @@ -125,10 +227,8 @@ serd_world_get_blank(SerdWorld* world) } void -serd_world_set_error_sink(SerdWorld* world, - SerdErrorSink error_sink, - void* handle) +serd_world_set_log_func(SerdWorld* world, SerdLogFunc log_func, void* handle) { - world->error_sink = error_sink; - world->error_handle = handle; + world->log_func = log_func; + world->log_handle = handle; } diff --git a/src/world.h b/src/world.h index 9402d65e..dc904cbc 100644 --- a/src/world.h +++ b/src/world.h @@ -19,13 +19,14 @@ #include "serd/serd.h" +#include #include #include struct SerdWorldImpl { SerdNodes* nodes; - SerdErrorSink error_sink; - void* error_handle; + SerdLogFunc log_func; + void* log_handle; SerdNode* blank_node; const SerdNode* rdf_first; const SerdNode* rdf_nil; @@ -39,9 +40,32 @@ struct SerdWorldImpl { FILE* serd_world_fopen(SerdWorld* world, const char* path, const char* mode); -SerdStatus serd_world_error(const SerdWorld* world, const SerdError* e); +/// Write a message to the log +SERD_API +SERD_LOG_FUNC(5, 0) +SerdStatus +serd_world_vlogf_internal(const SerdWorld* world, + SerdStatus st, + SerdLogLevel level, + const SerdCursor* cursor, + const char* fmt, + va_list args); +/// Write a message to the log +SERD_API +SERD_LOG_FUNC(5, 6) SerdStatus -serd_world_errorf(const SerdWorld* world, SerdStatus st, const char* fmt, ...); +serd_world_logf_internal(const SerdWorld* world, + SerdStatus st, + SerdLogLevel level, + const SerdCursor* cursor, + const char* fmt, + ...); + +#define SERD_LOG_ERRORF(world, st, fmt, ...) \ + serd_world_logf_internal(world, st, SERD_LOG_LEVEL_ERR, NULL, fmt, __VA_ARGS__); + +#define SERD_LOG_ERROR(world, st, msg) \ + serd_world_logf_internal(world, st, SERD_LOG_LEVEL_ERR, NULL, msg); #endif // SERD_WORLD_H diff --git a/src/writer.c b/src/writer.c index 86a5c966..fe77710a 100644 --- a/src/writer.c +++ b/src/writer.c @@ -110,8 +110,8 @@ struct SerdWriterImpl { SerdStack anon_stack; SerdWriteFunc write_func; void* stream; - SerdErrorSink error_sink; - void* msg_handle; + SerdLogFunc log_func; + void* log_handle; WriteContext context; unsigned indent; char* bprefix; @@ -180,8 +180,9 @@ write_character(SerdWriter* writer, const uint8_t* utf8, size_t* size) const uint32_t c = parse_utf8_char(utf8, size); switch (*size) { case 0: - serd_world_errorf( - writer->world, SERD_ERR_BAD_ARG, "invalid UTF-8: %X\n", utf8[0]); + SERD_LOG_ERRORF(writer->world, SERD_ERR_BAD_ARG, + "invalid UTF-8: %X\n", + utf8[0]); return sink(replacement_char, sizeof(replacement_char), writer); case 1: snprintf(escape, sizeof(escape), "\\u%04X", utf8[0]); @@ -585,10 +586,9 @@ write_curie(SerdWriter* const writer, case SERD_NQUADS: if ((st = serd_env_expand_in_place( writer->iface.env, node, &prefix, &suffix))) { - serd_world_errorf(writer->world, - st, - "undefined namespace prefix `%s'\n", - serd_node_get_string(node)); + SERD_LOG_ERRORF(writer->world, st, + "undefined namespace prefix `%s'\n", + serd_node_get_string(node)); return false; } sink("<", 1, writer); @@ -863,8 +863,8 @@ serd_writer_end_anon(SerdWriter* writer, if (writer->syntax == SERD_NTRIPLES || writer->syntax == SERD_NQUADS) { return SERD_SUCCESS; } else if (serd_stack_is_empty(&writer->anon_stack)) { - return serd_world_errorf(writer->world, SERD_ERR_UNKNOWN, - "unexpected end of anonymous node\n"); + return SERD_LOG_ERROR(writer->world, SERD_ERR_UNKNOWN, + "unexpected end of anonymous node\n"); } write_sep(writer, SEP_ANON_END); -- cgit v1.2.1