coova / coova-chilli

CoovaChilli is an open-source software access controller for captive portal hotspots.
Other
516 stars 258 forks source link

Implementing features: generic logging interface. #163

Open gbaligh opened 8 years ago

gbaligh commented 8 years ago

What do you think about an Implementation of generic logging interface. This implementation must be thread-safe, Level support, use syslog, enable log by module.

void chilli_log_init(chilli_log_t *log);
void chilli_log(char const *fmt, ...) __attribute__ ((__format__ (printf, 1, 2)));
void chilli_llog(chilli_log_t *log, unsigned level, char const *fmt, ...) __attribute__ ((__format__ (printf, 3, 4)));
void chilli_vllog(chilli_log_t *log, unsigned level,  char const *fmt, va_list ap);
void chilli_log_set_level(chilli_log_t *log, unsigned level);
ynezz commented 8 years ago

Yep, it's clear from #119, that we need to improve logging.

This implementation must be thread-safe, Level support, use syslog, enable log by module.

What do you mean by thread-safe exactly? Are you going to have different settings for every thread, are you going to share some buffers etc? Isn't it overdesigned for our needs?

How are you going to solve enable log by module by the proposed API? I don't see it from the API. Do we need this? Isn't just a log level verbosity good enough (ERROR, VERBOSE, DEBUG...)?

What are you going to keep in struct chilli_log_t?

I would simply use ulog modified for our needs (adding DEBUG log level, removing kmsg logging etc.).

gbaligh commented 8 years ago

Yes, you are totally right for Thread-safe capability.

struct chilli_log_s {
  char const  *log_name;
  unsigned  log_level;
  int  log_init;
};

/* Initialize a chilli_log_s structure */
#define CHILLI_LOG_INIT(name, level)  

#ifndef CHILLI_LOG
#define CHILLI_LOG       (chilli_log_default)
#else
struct chilli_log_s CHILLI_LOG[];
#endif

struct chilli_log_s chilli_log_default[];

void chilli_log(unsigned level, char const *fmt, ...) __attribute__ ((__format__ (printf, 1, 2))) 

/* in dhcp.c */
#define CHILLI_LOG_CTX  log_dhcp;
#include <chilli_log.h>
...
struct chilli_log_s log_dhcp[1] = { "DHCP", LOG_DEBUG, 1 };
...
chilli_log(LOG_INFO, "some INFO message");
/* Will print: [DHCP] - some INFO message */
...
chilli_log(LOG_DEBUG, "some DEBUG message");
/* Will print: [DHCP] - some DEBUG message */

/* in dns.c */
#define CHILLI_LOG_CTX  log_dns;
#include <chilli_log.h>
...
struct chilli_log_s log_dns[1] = { "DNS", LOG_DEBUG, 1 };
...
chilli_log(LOG_INFO, "some INFO message");
/* Will print: [DNS] - some INFO message */
...
chilli_log(LOG_DEBUG, "some DEBUG message");
/* Will print: [DNS] - some DEBUG message */