Finish rewriting configuration and logging code

Fix endless recursion if error or debug logged while reading config file
Fix 'config del' logic
Log messages made before log file can be opened are buffered and written
once the file is open
Do not log to file in ANDROID version, just to Android's log system
This commit is contained in:
Andrew Bettison 2012-07-05 14:59:50 +09:30
parent bbdfda2ce4
commit a80da50636
5 changed files with 89 additions and 52 deletions

55
conf.c
View File

@ -61,6 +61,7 @@ static char *config_buffer_end = NULL;
static unsigned int confc = 0;
static char *confvar[MAX_CONFIG_VARS];
static char *confvalue[MAX_CONFIG_VARS];
static int reading = 0;
static char *grow_config_buffer(size_t needed)
{
@ -70,7 +71,6 @@ static char *grow_config_buffer(size_t needed)
if (newsize > cursize) {
// Round up to nearest multiple of CONFIG_BUFFER_ALLOCSIZE.
newsize = newsize + CONFIG_BUFFER_ALLOCSIZE - ((newsize - 1) % CONFIG_BUFFER_ALLOCSIZE + 1);
DEBUGF("newsize=%llu", (unsigned long long) newsize);
char *newbuf = realloc(config_buffer, newsize);
if (newbuf == NULL) {
WHYF_perror("realloc(%llu)", newsize);
@ -91,12 +91,13 @@ static char *grow_config_buffer(size_t needed)
return ret;
}
static int read_config()
static int _read_config()
{
char conffile[1024];
if (!FORM_SERVAL_INSTANCE_PATH(conffile, "serval.conf"))
return -1;
size_t size = 0;
confc = 0;
FILE *f = fopen(conffile, "r");
if (f == NULL) {
if (errno != ENOENT)
@ -123,7 +124,6 @@ static int read_config()
fclose(f);
return -1;
}
confc = 0;
if (fread(config_buffer, size, 1, f) != 1) {
if (ferror(f))
WHYF_perror("fread(%s, %llu)", conffile, (unsigned long long) size);
@ -165,12 +165,12 @@ static int read_config()
while (c < e && *c != '\r' && *c != '\n')
++c;
if (c < e && *c == '\n') {
++confc;
*c++ = '\0';
++confc;
} else if (c < e - 1 && *c == '\r' && c[1] == '\n') {
*c++ = '\0';
*c++ = '\0';
++confc;
*c++ = '\0';
*c++ = '\0';
} else {
problem = "missing end-of-line";
}
@ -190,6 +190,26 @@ static int read_config()
return 0;
}
/* Set a flag while reading config, to avoid infinite recursion between here and logging
that could be caused by any WHY() or WARN() or DEBUG() invoked in _read_config(). The
problem is that on the first log message, the logging system calls confValueGet() to
discover the path of the log file, and that will return here.
*/
static int read_config()
{
if (reading)
return -1;
reading = 1;
int ret = _read_config();
reading = 0;
return ret;
}
int confLocked()
{
return reading;
}
int confVarCount()
{
if (!config_buffer && read_config() == -1)
@ -291,18 +311,18 @@ void confSetDebugFlags()
if (flag != -1) {
if (mask == DEBUG_ALL) {
if (flag) {
DEBUGF("Set all debug flags");
//DEBUGF("Set all debug flags");
setall = 1;
} else {
DEBUGF("Clear all debug flags");
//DEBUGF("Clear all debug flags");
clearall = 1;
}
} else {
if (flag) {
DEBUGF("Set %s", var);
//DEBUGF("Set %s", var);
setmask |= mask;
} else {
DEBUGF("Clear %s", var);
//DEBUGF("Clear %s", var);
clearmask |= mask;
}
}
@ -337,22 +357,22 @@ int confValueSet(const char *var, const char *value)
return WHYF("Cannot %s %s: invalid variable name", value ? "set" : "delete", var);
if (value == NULL) {
unsigned int i;
for (i = 0; i != confc; ++i)
DEBUGF("var=%s confvar[%u]=%s", var, i, confvar[i]);
for (i = 0; i < confc; ++i) {
if (strcasecmp(var, confvar[i]) == 0) {
DEBUGF("found");
--confc;
for (; i != confc; ++i) {
confvar[i] = confvar[i + i];
confvalue[i] = confvalue[i + i];
for (; i < confc; ++i) {
confvar[i] = confvar[i + 1];
confvalue[i] = confvalue[i + 1];
}
return 0;
}
}
} else {
if (confc >= MAX_CONFIG_VARS)
return WHYF("Cannot set %s: too many variables", var);
size_t valuelen = strlen(value);
unsigned int i;
for (i = 0; i != confc; ++i)
for (i = 0; i != confc; ++i) {
if (strcasecmp(var, confvar[i]) == 0) {
char *valueptr = confvalue[i];
if (valuelen > strlen(valueptr)) {
@ -363,6 +383,7 @@ int confValueSet(const char *var, const char *value)
confvalue[i] = strcpy(valueptr, value);
return 0;
}
}
size_t varlen = strlen(var);
char *buf = grow_config_buffer(varlen + 1 + valuelen + 1);
if (buf == NULL)

78
log.c
View File

@ -23,7 +23,12 @@ Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
unsigned int debug = 0;
static FILE *logfile = NULL;
static int opening = 0;
/* The logbuf is used to accumulate log messages before the log file is open and ready for
writing.
*/
static char _log_buf[8192];
static struct strbuf logbuf = STRUCT_STRBUF_EMPTY;
#ifdef ANDROID
#include <android/log.h>
@ -32,26 +37,24 @@ static int opening = 0;
FILE *open_logging()
{
if (!logfile) {
if (opening) {
const char *logpath = getenv("SERVALD_LOG_FILE");
if (!logpath) {
// If the configuration is locked (eg, it called WHY() or DEBUG() while initialising, which
// led back to here) then return NULL to indicate the message cannot be logged.
if (confLocked())
return NULL;
logpath = confValueGet("logfile", NULL);
}
if (!logpath) {
logfile = stderr;
INFO("Premature logging to stderr");
INFO("No logfile configured -- logging to stderr");
} else if ((logfile = fopen(logpath, "a"))) {
setlinebuf(logfile);
INFOF("Logging to %s (fd %d)", logpath, fileno(logfile));
} else {
++opening;
const char *logpath = getenv("SERVALD_LOG_FILE");
if (!logpath)
logpath = confValueGet("logfile", NULL);
if (!logpath) {
logfile = stderr;
INFO("No logfile configured -- logging to stderr");
} else if ((logfile = fopen(logpath, "a"))) {
setlinebuf(logfile);
INFOF("Logging to %s (fd %d)", logpath, fileno(logfile));
} else {
logfile = stderr;
WARN_perror("fopen");
WARNF("Cannot append to %s -- falling back to stderr", logpath);
}
--opening;
logfile = stderr;
WARN_perror("fopen");
WARNF("Cannot append to %s -- falling back to stderr", logpath);
}
}
return logfile;
@ -76,9 +79,22 @@ void logMessage(int level, const char *file, unsigned int line, const char *func
void vlogMessage(int level, const char *file, unsigned int line, const char *function, const char *fmt, va_list ap)
{
if (level != LOG_LEVEL_SILENT) {
strbuf b = strbuf_alloca(8192);
strbuf_sprintf(b, "%s:%u:%s() ", file ? trimbuildpath(file) : "NULL", line, function ? function : "NULL");
strbuf_vsprintf(b, fmt, ap);
if (strbuf_is_empty(&logbuf))
strbuf_init(&logbuf, _log_buf, sizeof _log_buf);
#ifndef ANDROID
const char *levelstr = "UNKNOWN";
switch (level) {
case LOG_LEVEL_FATAL: levelstr = "FATAL"; break;
case LOG_LEVEL_ERROR: levelstr = "ERROR"; break;
case LOG_LEVEL_INFO: levelstr = "INFO"; break;
case LOG_LEVEL_WARN: levelstr = "WARN"; break;
case LOG_LEVEL_DEBUG: levelstr = "DEBUG"; break;
}
strbuf_sprintf(&logbuf, "%s: [%d] ", levelstr, getpid());
#endif
strbuf_sprintf(&logbuf, "%s:%u:%s() ", file ? trimbuildpath(file) : "NULL", line, function ? function : "NULL");
strbuf_vsprintf(&logbuf, fmt, ap);
strbuf_puts(&logbuf, "\n");
#ifdef ANDROID
int alevel = ANDROID_LOG_UNKNOWN;
switch (level) {
@ -88,18 +104,14 @@ void vlogMessage(int level, const char *file, unsigned int line, const char *fun
case LOG_LEVEL_WARN: alevel = ANDROID_LOG_WARN; break;
case LOG_LEVEL_DEBUG: alevel = ANDROID_LOG_DEBUG; break;
}
__android_log_print(alevel, "servald", "%s", strbuf_str(b));
#endif
const char *levelstr = "UNKNOWN";
switch (level) {
case LOG_LEVEL_FATAL: levelstr = "FATAL"; break;
case LOG_LEVEL_ERROR: levelstr = "ERROR"; break;
case LOG_LEVEL_INFO: levelstr = "INFO"; break;
case LOG_LEVEL_WARN: levelstr = "WARN"; break;
case LOG_LEVEL_DEBUG: levelstr = "DEBUG"; break;
__android_log_print(alevel, "servald", "%s", strbuf_str(&logbuf));
#else
FILE *logf = open_logging();
if (logf) {
fputs(strbuf_str(&logbuf), logf);
strbuf_reset(&logbuf);
}
if (logfile || open_logging())
fprintf(logfile, "%s: [%d] %s\n", levelstr, getpid(), strbuf_str(b));
#endif
}
}

View File

@ -661,6 +661,7 @@ typedef struct overlay_txqueue {
#define OQ_MAX 5
extern overlay_txqueue overlay_tx[OQ_MAX];
int confLocked();
const char *confValueGet(const char *var, const char *defaultValue);
int confValueGetBoolean(const char *var, int defaultValue);
int64_t confValueGetInt64(const char *var, int64_t defaultValue);

View File

@ -532,6 +532,9 @@ int isDNAPacket(FILE *f,unsigned char *packet,int *ofs,int len)
int serval_packetvisualise(FILE *f,char *message,unsigned char *packet,int len)
{
if (f == NULL)
return -1;
if (message) fprintf(f,"%s:\n",message);
int i,j;

View File

@ -108,11 +108,11 @@ test_CaseInsensitive() {
assertStdoutGrep --stdout --stderr --matches=1 '^foo=bar$'
executeOk_servald config get Foo
assertStdoutLineCount '==' 1
assertStdoutGrep --stdout --stderr --matches=1 '^foo=bar$'
assertStdoutGrep --stdout --stderr --matches=1 '^Foo=bar$'
executeOk_servald config set FOO wah
executeOk_servald config get foo
assertStdoutLineCount '==' 1
assertStdoutGrep --stdout --stderr --matches=1 '^FOO=wah$'
assertStdoutGrep --stdout --stderr --matches=1 '^foo=wah$'
}
doc_DotsInNames="Config item names can have internal dots"