Skip to content

Commit 87f1979

Browse files
committed
Refactor logging: eliminate duplicated code
The _log() function was duplicating all the logging logic from logging_context_log(), making maintenance difficult and error-prone. Changes: - Created shared logging_vlog_impl() helper that takes va_list - Both logging_context_log() and _log() now call this helper - Removed 57 lines of duplicated code - No functional changes, just cleaner code structure This reduces maintenance burden and ensures consistent behavior between context-aware and legacy logging paths.
1 parent 27de2a6 commit 87f1979

File tree

1 file changed

+14
-70
lines changed

1 file changed

+14
-70
lines changed

src/logging.c

Lines changed: 14 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -105,14 +105,13 @@ void logging_context_flight_recorder_dump(logging_context_t *ctx) {
105105
}
106106
}
107107

108-
// NOLINTNEXTLINE(misc-no-recursion) because of severity check
109-
void logging_context_log(logging_context_t *ctx, const char *file, int line,
110-
int severity, const char *fmt, ...) {
108+
// Core logging implementation (used by both context-aware and legacy APIs)
109+
static void logging_vlog_impl(logging_context_t *ctx, const char *file, int line,
110+
int severity, const char *fmt, va_list args) {
111111
if (severity < ctx->loglevel && !ctx->flight_recorder) {
112112
return;
113113
}
114114
if (severity < 0 || severity >= LOG_MAX) {
115-
// Can't use FLOG here due to recursion, just log error
116115
fprintf(stderr, "Unknown log severity: %d\n", severity);
117116
return;
118117
}
@@ -132,10 +131,7 @@ void logging_context_log(logging_context_t *ctx, const char *file, int line,
132131
}
133132
buff_pos += (uint32_t)chars;
134133

135-
va_list args;
136-
va_start(args, fmt);
137134
chars = vsnprintf(buff + buff_pos, LOG_LINE_SIZE - buff_pos, fmt, args); // NOLINT(clang-diagnostic-format-nonliteral)
138-
va_end(args);
139135

140136
if (chars < 0) {
141137
abort(); // must be impossible
@@ -170,6 +166,15 @@ void logging_context_log(logging_context_t *ctx, const char *file, int line,
170166
}
171167
}
172168

169+
// NOLINTNEXTLINE(misc-no-recursion) because of severity check
170+
void logging_context_log(logging_context_t *ctx, const char *file, int line,
171+
int severity, const char *fmt, ...) {
172+
va_list args;
173+
va_start(args, fmt);
174+
logging_vlog_impl(ctx, file, line, severity, fmt, args);
175+
va_end(args);
176+
}
177+
173178
// === Legacy API (uses default global context) ===
174179

175180
logging_context_t* logging_get_default_context(void) {
@@ -207,76 +212,15 @@ void logging_flight_recorder_dump(void) {
207212
logging_context_flight_recorder_dump(&g_default_context);
208213
}
209214

210-
// Keep original _log for backwards compatibility with existing macros
215+
// Legacy _log function for backwards compatibility with existing macros
211216
void _log(const char *file, int line, int severity, const char *fmt, ...) {
212217
if (!g_default_initialized) {
213218
// Auto-initialize to stderr if not already initialized
214219
logging_init(STDERR_FILENO, LOG_ERROR, 0);
215220
}
216221

217-
if (severity < g_default_context.loglevel && !g_default_context.flight_recorder) {
218-
return;
219-
}
220-
221-
// Forward to context-aware implementation
222222
va_list args;
223223
va_start(args, fmt);
224-
225-
// Manually inline the logging logic since we can't forward va_list easily
226-
if (severity < 0 || severity >= LOG_MAX) {
227-
fprintf(stderr, "Unknown log severity: %d\n", severity);
228-
va_end(args);
229-
return;
230-
}
231-
232-
if (!g_default_context.logfile) {
233-
g_default_context.logfile = fdopen(STDOUT_FILENO, "w");
234-
}
235-
236-
struct timeval tv;
237-
gettimeofday(&tv, NULL);
238-
239-
char buff[LOG_LINE_SIZE];
240-
uint32_t buff_pos = 0;
241-
int chars = snprintf(buff, LOG_LINE_SIZE, "%s %8"PRIu64".%06"PRIu64" %s:%d ",
242-
SeverityStr[severity], (uint64_t)tv.tv_sec, (uint64_t)tv.tv_usec, file, line);
243-
if (chars < 0 || chars >= LOG_LINE_SIZE/2) {
244-
abort();
245-
}
246-
buff_pos += (uint32_t)chars;
247-
248-
chars = vsnprintf(buff + buff_pos, LOG_LINE_SIZE - buff_pos, fmt, args); // NOLINT(clang-diagnostic-format-nonliteral)
224+
logging_vlog_impl(&g_default_context, file, line, severity, fmt, args);
249225
va_end(args);
250-
251-
if (chars < 0) {
252-
abort();
253-
}
254-
buff_pos += (uint32_t)chars;
255-
if (buff_pos >= LOG_LINE_SIZE) {
256-
buff_pos = LOG_LINE_SIZE - 1;
257-
buff[buff_pos - 1] = '$';
258-
}
259-
260-
if (g_default_context.flight_recorder) {
261-
ring_buffer_push_back(g_default_context.flight_recorder, buff, buff_pos);
262-
}
263-
264-
if (severity < g_default_context.loglevel) {
265-
return;
266-
}
267-
(void)fprintf(g_default_context.logfile, "%s\n", buff);
268-
269-
if (severity >= LOG_FLUSH_LEVEL) {
270-
(void)fflush(g_default_context.logfile);
271-
}
272-
if (severity == LOG_FATAL) {
273-
if (g_default_context.flight_recorder) {
274-
ring_buffer_dump(g_default_context.flight_recorder, g_default_context.logfile);
275-
}
276-
#ifdef DEBUG
277-
abort();
278-
#else
279-
exit(1);
280-
#endif
281-
}
282226
}

0 commit comments

Comments
 (0)