Fix bugs in earlier conf/log recursion logic

Reinstate better recursion control in conf.c, remove from log.c.

Add <kludge> comment to "config set" code, add the same kludge to "config
del".
This commit is contained in:
Andrew Bettison 2012-11-15 12:38:17 +10:30
parent f2d6c6d522
commit 6395f757cc
4 changed files with 118 additions and 60 deletions

View File

@ -900,7 +900,21 @@ int app_config_set(int argc, const char *const *argv, struct command_line_option
return -1;
if (create_serval_instance_dir() == -1)
return -1;
confReloadIfNewer();
// <kludge>
// This fixes a subtle bug in when upgrading the Batphone app: the servald.conf file does
// not get upgraded. The bug goes like this:
// 1. new Batphone APK is installed, but prior servald.conf is not overwritten because it
// comes in serval.zip;
// 2. new Batphone is started, which calls JNI "stop" command, which reads the old servald.conf
// into memory buffer;
// 3. new Batphone unpacks serval.zip, overwriting servald.conf with new version;
// 4. new Batphone calls JNI "config set rhizome.enable 1", which sets the "rhizome.enable"
// config option in the existing memory buffer and overwrites servald.conf;
// Bingo, the old version of servald.conf is what remains. This kludge intervenes in step 4, by
// reading the new servald.conf into the memory buffer before applying the "rhizome.enable" set
// value and overwriting.
confReloadIfChanged();
// </kludge>
return confValueSet(var, val) == -1 ? -1 : confWrite();
}
@ -912,6 +926,9 @@ int app_config_del(int argc, const char *const *argv, struct command_line_option
return -1;
if (create_serval_instance_dir() == -1)
return -1;
// <kludge> See app_config_set()
confReloadIfChanged();
// </kludge>
return confValueSet(var, NULL) == -1 ? -1 : confWrite();
}

109
conf.c
View File

@ -58,6 +58,7 @@ int is_configvarname(const char *arg)
#define MAX_CONFIG_VARS (100)
#define CONFIG_BUFFER_ALLOCSIZE (1024)
static int busy = 0;
static time_t config_timestamp = 0;
static char *config_buffer = NULL;
static char *config_buffer_top = NULL;
@ -66,6 +67,9 @@ static unsigned int confc = 0;
static char *confvar[MAX_CONFIG_VARS];
static char *confvalue[MAX_CONFIG_VARS];
#define BUSY_BODY(_type, _return_if_busy, _return) \
do { if (busy) { return (_return_if_busy); } else { busy = 1; _type ret = (_return); busy = 0; return ret; } } while (0)
static char *grow_config_buffer(size_t needed)
{
size_t cursize = config_buffer_end - config_buffer;
@ -208,7 +212,18 @@ static int _read_config()
static int read_config()
{
return _read_config();
BUSY_BODY(int, -1, _read_config());
}
/* Return true if any conf...() function is in progress, ie, a configuration fetch function is being
* re-entered. This function allows other modules to avoid this re-entry, typically the logging
* system that uses confValueGet() to set itself up, which in turn can log messages.
*
* @author Andrew Bettison <andrew@servalproject.com>
*/
int confBusy()
{
return busy;
}
/* Check if the config file has changed since we last read it, and if so, invalidate the buffer so
@ -220,7 +235,7 @@ static int read_config()
*
* @author Andrew Bettison <andrew@servalproject.com>
*/
int confReloadIfNewer()
static int _confReloadIfChanged()
{
char conffile[1024];
if (!FORM_SERVAL_INSTANCE_PATH(conffile, CONFFILE_NAME))
@ -238,17 +253,46 @@ int confReloadIfNewer()
}
return 0;
}
int confReloadIfChanged()
{
BUSY_BODY(int, -1, _confReloadIfChanged());
}
/* Return the number of config options.
*
* @author Andrew Bettison <andrew@servalproject.com>
*/
int confVarCount()
static int _confVarCount()
{
if (!config_buffer && read_config() == -1)
if (!config_buffer && _read_config() == -1)
return -1;
return confc;
}
int confVarCount()
{
BUSY_BODY(int, -1, _confVarCount());
}
/* Return the string name of the config option with the given index, which must be in the range
* 0..confVarCount(). The returned pointer is only valid until the next call to confVar() or any
* other configuration query function.
*
* @author Andrew Bettison <andrew@servalproject.com>
*/
static const char *_confVar(unsigned int index)
{
if (!config_buffer && _read_config() == -1)
return NULL;
if (index >= confc) {
WHYF("Config index=%u too big, confc=%u", index, confc);
return NULL;
}
return confvar[index];
}
const char *confVar(unsigned int index)
{
BUSY_BODY(const char *, NULL, _confVar(index));
}
/* Return the string value of the config option with the given index, which must be in the range
* 0..confVarCount(). The returned pointer is only valid until the next call to confVar() or any
@ -256,20 +300,9 @@ int confVarCount()
*
* @author Andrew Bettison <andrew@servalproject.com>
*/
const char *confVar(unsigned int index)
static const char *_confValue(unsigned int index)
{
if (!config_buffer && read_config() == -1)
return NULL;
if (index >= confc) {
WHYF("Config index=%u too big, confc=%u", index, confc);
return NULL;
}
return confvar[index];
}
const char *confValue(unsigned int index)
{
if (!config_buffer && read_config() == -1)
if (!config_buffer && _read_config() == -1)
return NULL;
if (index >= confc) {
WHYF("Config index=%u too big, confc=%u", index, confc);
@ -277,6 +310,10 @@ const char *confValue(unsigned int index)
}
return confvalue[index];
}
const char *confValue(unsigned int index)
{
BUSY_BODY(const char *, NULL, _confValue(index));
}
/* Return the string value of the config option with the given name. The returned pointer is only
* valid until the next call to confVarCount(), confVar(), confValue() or any other configuration
@ -286,13 +323,13 @@ const char *confValue(unsigned int index)
*
* @author Andrew Bettison <andrew@servalproject.com>
*/
const char *confValueGet(const char *var, const char *defaultValue)
static const char *_confValueGet(const char *var, const char *defaultValue)
{
if (var == NULL) {
WHYF("NULL var name, returning default value: %s", defaultValue ? defaultValue : "NULL");
return defaultValue;
}
if (!config_buffer && read_config() == -1) {
if (!config_buffer && _read_config() == -1) {
if (defaultValue)
WARNF("Config option %s: using default value: %s", var, defaultValue);
return defaultValue;
@ -303,10 +340,14 @@ const char *confValueGet(const char *var, const char *defaultValue)
return confvalue[i];
return defaultValue;
}
int confValueGetBoolean(const char *var, int defaultValue)
const char *confValueGet(const char *var, const char *defaultValue)
{
const char *value = confValueGet(var, NULL);
BUSY_BODY(const char *, defaultValue, _confValueGet(var, defaultValue));
}
static int _confValueGetBoolean(const char *var, int defaultValue)
{
const char *value = _confValueGet(var, NULL);
if (!value)
return defaultValue;
int flag = confParseBoolean(value, var);
@ -315,10 +356,14 @@ int confValueGetBoolean(const char *var, int defaultValue)
WARNF("Config option %s: using default value %s", var, defaultValue ? "true" : "false");
return defaultValue;
}
int64_t confValueGetInt64(const char *var, int64_t defaultValue)
int confValueGetBoolean(const char *var, int defaultValue)
{
const char *start = confValueGet(var, NULL);
BUSY_BODY(int, defaultValue, _confValueGetBoolean(var, defaultValue));
}
static int64_t _confValueGetInt64(const char *var, int64_t defaultValue)
{
const char *start = _confValueGet(var, NULL);
if (!start)
return defaultValue;
const char *end = start;
@ -328,16 +373,24 @@ int64_t confValueGetInt64(const char *var, int64_t defaultValue)
WARNF("Config option %s: '%s' is not an integer, using default value %lld", var, start, (long long) defaultValue);
return defaultValue;
}
int64_t confValueGetInt64Range(const char *var, int64_t defaultValue, int64_t rangemin, int64_t rangemax)
int64_t confValueGetInt64(const char *var, int64_t defaultValue)
{
int64_t value = confValueGetInt64(var, defaultValue);
BUSY_BODY(int64_t, defaultValue, _confValueGetInt64(var, defaultValue));
}
static int64_t _confValueGetInt64Range(const char *var, int64_t defaultValue, int64_t rangemin, int64_t rangemax)
{
int64_t value = _confValueGetInt64(var, defaultValue);
if (value >= rangemin || value <= rangemax)
return value;
WARNF("Config option %s: configured value %lld out of range [%lld,%lld], using default value %lld",
var, (long long) value, (long long) rangemin, (long long) rangemax, (long long) defaultValue);
return defaultValue;
}
int64_t confValueGetInt64Range(const char *var, int64_t defaultValue, int64_t rangemin, int64_t rangemax)
{
BUSY_BODY(int64_t, defaultValue, _confValueGetInt64Range(var, defaultValue, rangemin, rangemax));
}
void confSetDebugFlags()
{

3
conf.h
View File

@ -35,7 +35,8 @@ Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*/
#define FORM_SERVAL_INSTANCE_PATH(buf, path) (form_serval_instance_path(buf, sizeof(buf), (path)))
int confReloadIfNewer();
int confBusy();
int confReloadIfChanged();
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);

47
log.c
View File

@ -44,7 +44,6 @@ const struct __sourceloc __whence = __NOWHERE__;
debugflags_t debug = 0;
static int busy = 0;
static FILE *logfile = NULL;
static int flag_show_pid = -1;
static int flag_show_time = -1;
@ -78,21 +77,19 @@ static FILE *_open_logging()
if (!logfile) {
const char *logpath = getenv("SERVALD_LOG_FILE");
if (!logpath) {
// If the logging system is currently busy (eg, confValueGet() logged a message, which
// re-entered here) then return NULL to indicate the message cannot be logged.
if (busy > 1)
if (confBusy())
return NULL;
logpath = confValueGet("log.file", NULL);
}
if (!logpath) {
logfile = stderr;
logfile = stderr; //fopen("/tmp/foo", "a");
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");
logfile = stderr; //fopen("/tmp/bar", "a");
WARNF_perror("fopen(%s)", logpath);
WARNF("Cannot append to %s -- falling back to stderr", logpath);
}
}
@ -101,30 +98,20 @@ static FILE *_open_logging()
FILE *open_logging()
{
++busy;
FILE *ret = _open_logging();
assert(busy);
--busy;
return ret;
return _open_logging();
}
static int show_pid()
{
if (flag_show_pid < 0) {
if (busy > 1)
return 0;
if (flag_show_pid < 0 && !confBusy())
flag_show_pid = confValueGetBoolean("log.show_pid", 0);
}
return flag_show_pid;
}
static int show_time()
{
if (flag_show_time < 0) {
if (busy > 1)
return 0;
if (flag_show_time < 0 && !confBusy())
flag_show_time = confValueGetBoolean("log.show_time", 0);
}
return flag_show_time;
}
@ -154,14 +141,17 @@ static int _log_prepare(int level, struct __sourceloc whence)
{
if (level == LOG_LEVEL_SILENT)
return 0;
++busy;
struct timeval tv;
tv.tv_sec = 0;
int showtime = show_time();
if (showtime)
gettimeofday(&tv, NULL);
int showpid = show_pid();
_open_logging(); // Put initial INFO message at start of log file
// No calls outside log.c from this point on.
if (strbuf_is_empty(&logbuf))
strbuf_init(&logbuf, _log_buf, sizeof _log_buf);
_open_logging(); // Put initial INFO message at start of log file
int showpid = show_pid();
int showtime = show_time();
// No calls outside log.c from this point on.
if (strbuf_len(&logbuf))
else if (strbuf_len(&logbuf))
strbuf_putc(&logbuf, '\n');
#ifndef ANDROID
const char *levelstr = "UNKWN:";
@ -177,8 +167,7 @@ static int _log_prepare(int level, struct __sourceloc whence)
if (showpid)
strbuf_sprintf(&logbuf, "[%5u] ", getpid());
if (showtime) {
struct timeval tv;
if (gettimeofday(&tv, NULL) == -1) {
if (tv.tv_sec == 0) {
strbuf_puts(&logbuf, "NOTIME______ ");
} else {
struct tm tm;
@ -231,8 +220,6 @@ static void _log_finish(int level)
{
if (_log_implementation)
_log_implementation(level, &logbuf);
assert(busy);
--busy;
}
void set_log_implementation(void (*log_function)(int level, struct strbuf *buf))