Interface of skygw_logmanager_init(int argc, char* argv[]) changed.

The previous interface of skygw_logmanager_init was conceptually
broken. With -o you could specify that logging should be done to
stdout. However, even if you did that, the log manager still checked
that the logging directory could be accessed. Unless it had been
specified using -j <path> the default was /var/log/maxscale.

That is, unless the program calling skygw_logmanager_init was invoked
by a user that had write access to /var/log/maxscale, there would be
a complaint even if nothing was ever written to that directory.
In practice this meant that even if -o was used you had to provide
a -j with a path that surely is writeable (e.g. "/tmp").

This has now been changed so that you explicitly must provide the
log directory and the flags -j and -o are removed.

  bool skygw_logmanager_init(const char* logdir, int argc, char* argv[]);

If /logdir/ is provided then logged messages are written to a log file
in that directory. If /logdir/ is NULL then messages are logged to stdout
and no checks for access to any directory is not made.
This commit is contained in:
Johan Wikman 2015-11-06 12:29:43 +02:00
parent b19a4b9f4a
commit 450078fa92
11 changed files with 92 additions and 110 deletions

View File

@ -267,14 +267,14 @@ static bool filewriter_init(logmanager_t* logmanager,
skygw_message_t* clientmes,
skygw_message_t* logmes);
static void filewriter_done(filewriter_t* filewriter);
static bool fnames_conf_init(fnames_conf_t* fn, int argc, char* argv[]);
static bool fnames_conf_init(fnames_conf_t* fn, const char* logdir, int argc, char* argv[]);
static void fnames_conf_done(fnames_conf_t* fn);
static void fnames_conf_free_memory(fnames_conf_t* fn);
static void* thr_filewriter_fun(void* data);
static logfile_t* logmanager_get_logfile(logmanager_t* lm);
static bool logmanager_register(bool writep);
static void logmanager_unregister(void);
static bool logmanager_init_nomutex(int argc, char* argv[]);
static bool logmanager_init_nomutex(const char* logdir, int argc, char* argv[]);
static void logmanager_done_nomutex(void);
static bool logmanager_is_valid_id(logfile_id_t id);
@ -310,7 +310,7 @@ const char* get_logpath_default(void)
return "/var/log/maxscale";
}
static bool logmanager_init_nomutex(int argc, char* argv[])
static bool logmanager_init_nomutex(const char* logdir, int argc, char* argv[])
{
fnames_conf_t* fn;
filewriter_t* fw;
@ -356,7 +356,7 @@ static bool logmanager_init_nomutex(int argc, char* argv[])
}
/** Initialize configuration including log file naming info */
if (!fnames_conf_init(fn, argc, argv))
if (!fnames_conf_init(fn, logdir, argc, argv))
{
err = 1;
goto return_succp;
@ -418,15 +418,14 @@ return_succp:
/**
* Initializes log managing routines in MariaDB Corporation MaxScale.
*
* Parameters:
* @param logdir The directory for the log file. If NULL logging will be made to stdout.
* @param argc number of arguments in argv array
*
* @param argv arguments array
*
* @return true if succeed, otherwise false
*
*/
bool skygw_logmanager_init(int argc, char* argv[])
bool skygw_logmanager_init(const char* logdir, int argc, char* argv[])
{
bool succp = false;
@ -438,7 +437,7 @@ bool skygw_logmanager_init(int argc, char* argv[])
goto return_succp;
}
succp = logmanager_init_nomutex(argc, argv);
succp = logmanager_init_nomutex(logdir, argc, argv);
return_succp:
release_lock(&lmlock);
@ -1623,7 +1622,8 @@ static bool logmanager_register(bool writep)
if (lm == NULL)
{
succp = logmanager_init_nomutex(0, NULL);
// TODO: This looks fishy.
succp = logmanager_init_nomutex(get_logpath_default(), 0, NULL);
}
}
/** if logmanager existed or was succesfully restarted, increase link */
@ -1670,23 +1670,18 @@ static void logmanager_unregister(void)
* @node Initialize log file naming parameters from call arguments
* or from default functions in cases where arguments are not provided.
*
* Parameters:
* @param fn - <usage>
* <description>
*
* @param argc - <usage>
* <description>
*
* @param argv - <usage>
* <description>
*
* @return pointer to object which is either in RUN or DONE state.
* @param fn The fnames_conf_t structure to initialize.
* @param logdir The directory for the log file. If NULL logging will be made to stdout.
* @param argc number of arguments in argv array
* @param argv arguments array
*
* @return True if the initialization was performed, false otherwise.
*
* @details Note that input parameter lenghts are checked here.
*
*/
static bool fnames_conf_init(fnames_conf_t* fn,
const char* logdir,
int argc,
char* argv[])
{
@ -1694,11 +1689,9 @@ static bool fnames_conf_init(fnames_conf_t* fn,
bool succp = false;
const char* argstr =
"-h - help\n"
"-j <log path> ............(\"/tmp\")\n"
"-l <syslog log file ids> .......(no default)\n"
"-m <syslog ident> ............(argv[0])\n"
"-s <shmem log file ids> .......(no default)\n"
"-o .......(write logs to stdout)\n";
"-s <shmem log file ids> .......(no default)\n";
/**
* When init_started is set, clean must be done for it.
@ -1709,18 +1702,11 @@ static bool fnames_conf_init(fnames_conf_t* fn,
fn->fn_chk_tail = CHK_NUM_FNAMES;
#endif
optind = 1; /**<! reset getopt index */
while ((opt = getopt(argc, argv, "+h:j:l:m:s:o")) != -1)
while ((opt = getopt(argc, argv, "+h:l:m:s:")) != -1)
{
switch (opt)
{
case 'o':
use_stdout = 1;
break;
case 'j':
fn->fn_logpath = strndup(optarg, MAX_PATHLEN);
break;
case 'l':
/** record list of log file ids for syslogged */
if (do_syslog)
@ -1751,8 +1737,18 @@ static bool fnames_conf_init(fnames_conf_t* fn,
goto return_conf_init;
} /** switch (opt) */
}
/** If log file name is not specified in call arguments, use default. */
fn->fn_logpath = (fn->fn_logpath == NULL) ? strdup(get_logpath_default()) : fn->fn_logpath;
if (logdir)
{
use_stdout = 0;
fn->fn_logpath = strdup(logdir);
}
else
{
use_stdout = 1;
// TODO: Re-arrange things so that fn->fn_logpath can be NULL.
fn->fn_logpath = strdup(get_logpath_default());
}
/** Set identity string for syslog if it is not set in config.*/
if (do_syslog)
@ -1907,6 +1903,14 @@ static bool logfile_create(logfile_t* lf)
bool succp;
strpart_t spart[3]; /*< string parts of which the file is composed of */
if (use_stdout)
{
// TODO: Refactor so that lf_full_file_name can be NULL in this case.
lf->lf_full_file_name = strdup("stdout");
succp = true;
// TODO: Refactor to get rid of the gotos.
goto return_succp;
}
/**
* sparts is an array but next pointers are used to walk through
* the list of string parts.

View File

@ -145,7 +145,7 @@ int mxs_log_rotate();
int mxs_log_enable_priority(int priority);
int mxs_log_disable_priority(int priority);
bool skygw_logmanager_init(int argc, char* argv[]);
bool skygw_logmanager_init(const char* logdir, int argc, char* argv[]);
void skygw_logmanager_done(void);
void skygw_logmanager_exit(void);

View File

@ -105,7 +105,7 @@ int main(int argc, char* argv[])
fprintf(stderr, "Couldn't register exit function.\n");
}
succp = skygw_logmanager_init( log_argc, log_argv);
succp = skygw_logmanager_init("/tmp", log_argc, log_argv);
if(!succp)
fprintf(stderr, "Log manager initialization failed.\n");
ss_dassert(succp);
@ -121,7 +121,7 @@ int main(int argc, char* argv[])
tm.tm_min,
tm.tm_sec);
skygw_logmanager_init( log_argc, log_argv);
skygw_logmanager_init("/tmp", log_argc, log_argv);
logstr = ("First write with flush.");
err = skygw_log_write_flush(LOGFILE_ERROR, logstr);
@ -169,7 +169,7 @@ int main(int argc, char* argv[])
logstr = "Ph%dlip.";
err = skygw_log_write(LOGFILE_TRACE, logstr, 1);
skygw_logmanager_init( log_argc, log_argv);
skygw_logmanager_init("/tmp", log_argc, log_argv);
logstr = ("A terrible error has occurred!");
err = skygw_log_write_flush(LOGFILE_ERROR, logstr);
@ -301,7 +301,7 @@ int main(int argc, char* argv[])
err = skygw_log_write(LOGFILE_ERROR, logstr);
ss_dassert(err == 0);
succp = skygw_logmanager_init(log_argc, log_argv);
succp = skygw_logmanager_init("/tmp", log_argc, log_argv);
ss_dassert(succp);
skygw_log_disable(LOGFILE_TRACE);
@ -362,7 +362,7 @@ int main(int argc, char* argv[])
#endif /* TEST 3 */
#if defined(TEST4)
succp = skygw_logmanager_init(log_argc, log_argv);
succp = skygw_logmanager_init("/tmp", log_argc, log_argv);
ss_dassert(succp);
#if !defined(SS_DEBUG)
skygw_log_enable(LOGFILE_TRACE);
@ -397,7 +397,7 @@ int main(int argc, char* argv[])
skygw_logmanager_done();
succp = skygw_logmanager_init(log_argc, log_argv);
succp = skygw_logmanager_init("/tmp", log_argc, log_argv);
ss_dassert(succp);
#if !defined(SS_DEBUG)
skygw_log_enable(LOGFILE_TRACE);
@ -473,7 +473,7 @@ static void* thr_run(
char* logstr;
int err;
skygw_logmanager_init( 0, NULL);
skygw_logmanager_init("/tmp", 0, NULL);
skygw_logmanager_done();
skygw_log_flush(LOGFILE_MESSAGE);
logstr = ("Hi, how are you?");
@ -489,14 +489,14 @@ static void* thr_run(
fprintf(stderr,"Error, log write failed.\n");
ss_dassert(err == 0);
err = skygw_log_write(LOGFILE_MESSAGE, logstr);
skygw_logmanager_init( 0, NULL);
skygw_logmanager_init("/tmp", 0, NULL);
logstr = ("Testing. One, two, three\n");
err = skygw_log_write(LOGFILE_ERROR, logstr);
if(err != 0)
fprintf(stderr,"Error, log write failed.\n");
ss_dassert(err == 0);
skygw_logmanager_init( 0, NULL);
skygw_logmanager_init( 0, NULL);
skygw_logmanager_init("/tmp", 0, NULL);
skygw_logmanager_init("/tmp", 0, NULL);
skygw_log_flush(LOGFILE_ERROR);
logstr = ("For automatic and register variables, it is done each time the function or block is entered.");
@ -506,13 +506,13 @@ static void* thr_run(
fprintf(stderr,"Error, log write failed.\n");
ss_dassert(err == 0);
skygw_logmanager_done();
skygw_logmanager_init( 0, NULL);
skygw_logmanager_init("/tmp", 0, NULL);
logstr = ("Rather more surprising, at least at first sight, is the fact that a reference to a[i] can also be written as *(a+i). In evaluating a[i], C converts it to *(a+i) immediately; the two forms are equivalent. Applying the operatos & to both parts of this equivalence, it follows that &a[i] and a+i are also identical: a+i is the address of the i-th element beyond a.");
err = skygw_log_write(LOGFILE_ERROR, logstr);
if(err != 0)
fprintf(stderr,"Error, log write failed.\n");
ss_dassert(err == 0);
skygw_logmanager_init( 0, NULL);
skygw_logmanager_init("/tmp", 0, NULL);
skygw_logmanager_done();
skygw_log_flush(LOGFILE_ERROR);
skygw_logmanager_done();
@ -522,8 +522,8 @@ static void* thr_run(
if(err != 0)
fprintf(stderr,"Error, log write failed.\n");
ss_dassert(err == 0);
skygw_logmanager_init( 0, NULL);
skygw_logmanager_init( 0, NULL);
skygw_logmanager_init("/tmp", 0, NULL);
skygw_logmanager_init("/tmp", 0, NULL);
logstr = ("For automatic and register variables, it is done each time the function or block is entered.");
#if !defined(SS_DEBUG)
skygw_log_enable(LOGFILE_TRACE);
@ -532,13 +532,13 @@ static void* thr_run(
if(err != 0)
fprintf(stderr,"Error, log write failed.\n");
ss_dassert(err == 0);
skygw_logmanager_init( 0, NULL);
skygw_logmanager_init("/tmp", 0, NULL);
logstr = ("Rather more surprising, at least at first sight, is the fact that a reference to a[i] can also be written as *(a+i). In evaluating a[i], C converts it to *(a+i) immediately; the two forms are equivalent. Applying the operatos & to both parts of this equivalence, it follows that &a[i] and a+i are also identical: a+i is the address of the i-th element beyond a.");
err = skygw_log_write(LOGFILE_ERROR, logstr);
if(err != 0)
fprintf(stderr,"Error, log write failed.\n");
ss_dassert(err == 0);
skygw_logmanager_init( 0, NULL);
skygw_logmanager_init("/tmp", 0, NULL);
logstr = ("..... and you too?");
err = skygw_log_write(LOGFILE_MESSAGE, logstr);
if(err != 0)
@ -563,14 +563,14 @@ static void* thr_run(
if(err != 0)
fprintf(stderr,"Error, log write failed.\n");
ss_dassert(err == 0);
skygw_logmanager_init( 0, NULL);
skygw_logmanager_init("/tmp", 0, NULL);
logstr = ("Testing. One, two, three, .. where was I?\n");
err = skygw_log_write(LOGFILE_ERROR, logstr);
if(err != 0)
fprintf(stderr,"Error, log write failed.\n");
ss_dassert(err == 0);
skygw_logmanager_init( 0, NULL);
skygw_logmanager_init( 0, NULL);
skygw_logmanager_init("/tmp", 0, NULL);
skygw_logmanager_init("/tmp", 0, NULL);
skygw_logmanager_done();
simple_mutex_lock(td->mtx, true);
*td->nactive -= 1;

View File

@ -63,14 +63,12 @@ int main(int argc, char** argv)
sprintf(tmp,"%s",cwd);
optstr[0] = strdup("log_manager");
optstr[1] = strdup("-j");
optstr[2] = strdup(tmp);
optstr[3] = NULL;
optstr[1] = NULL;
iterations = atoi(argv[1]);
interval = atoi(argv[2]);
succp = skygw_logmanager_init( 3, optstr);
succp = skygw_logmanager_init(tmp, 1, optstr);
if(!succp)
fprintf(stderr,"Error, log manager initialization failed.\n");
ss_dassert(succp);

View File

@ -1716,8 +1716,6 @@ int main(int argc, char **argv)
}
argv[0] = "MaxScale";
argv[1] = "-j";
argv[2] = get_logdir();
if (!(*syslog_enabled))
{
@ -1733,21 +1731,21 @@ int main(int argc, char **argv)
if (logtofile)
{
argv[3] = "-l"; /*< write to syslog */
argv[1] = "-l"; /*< write to syslog */
/** Logs that should be syslogged */
argv[4] = "LOGFILE_MESSAGE,LOGFILE_ERROR"
argv[2] = "LOGFILE_MESSAGE,LOGFILE_ERROR"
"LOGFILE_DEBUG,LOGFILE_TRACE";
argv[5] = NULL;
succp = skygw_logmanager_init(5, argv);
argv[3] = NULL;
succp = skygw_logmanager_init(get_logdir(), 3, argv);
}
else
{
argv[3] = "-s"; /*< store to shared memory */
argv[4] = "LOGFILE_DEBUG,LOGFILE_TRACE"; /*< to shm */
argv[5] = "-l"; /*< write to syslog */
argv[6] = "LOGFILE_MESSAGE,LOGFILE_ERROR"; /*< to syslog */
argv[7] = NULL;
succp = skygw_logmanager_init(7, argv);
argv[1] = "-s"; /*< store to shared memory */
argv[2] = "LOGFILE_DEBUG,LOGFILE_TRACE"; /*< to shm */
argv[3] = "-l"; /*< write to syslog */
argv[4] = "LOGFILE_MESSAGE,LOGFILE_ERROR"; /*< to syslog */
argv[5] = NULL;
succp = skygw_logmanager_init(get_logdir(), 5, argv);
}
if (!succp)

View File

@ -35,7 +35,7 @@
int main(int argc, char **argv)
{
int arg_count = 4;
int arg_count = 1;
char *home;
char *keyfile;
char** arg_vector;
@ -59,11 +59,8 @@ int main(int argc, char **argv)
}
arg_vector[0] = "logmanager";
arg_vector[1] = "-j";
arg_vector[2] = "/var/log/maxscale/maxkeys";
arg_vector[3] = "-o";
arg_vector[4] = NULL;
skygw_logmanager_init(arg_count, arg_vector);
arg_vector[1] = NULL;
skygw_logmanager_init(NULL, arg_count, arg_vector);
free(arg_vector);
if (secrets_writeKeys(keyfile))

View File

@ -42,7 +42,7 @@ main(int argc, char **argv)
{
char *enc;
char *pw;
int arg_count = 6;
int arg_count = 1;
char *home;
char** arg_vector;
int rval = 0;
@ -61,16 +61,9 @@ main(int argc, char **argv)
return 1;
}
arg_vector[0] = strdup("logmanager");
arg_vector[1] = strdup("-j");
arg_vector[2] = strdup("/var/log/maxscale");
arg_vector[3] = "-o";
arg_vector[4] = "-l";
arg_vector[5] = "LOGFILE_ERROR";
arg_vector[6] = NULL;
skygw_logmanager_init(arg_count, arg_vector);
free(arg_vector[2]);
arg_vector[0] = "logmanager";
arg_vector[1] = NULL;
skygw_logmanager_init(NULL, arg_count, arg_vector);
free(arg_vector);
pw = calloc(81, sizeof(char));

View File

@ -8,19 +8,19 @@
void init_test_env(char *path)
{
int argc = 5;
int argc = 3;
const char* logdir = path ? path : TEST_LOG_DIR;
char* argv[] =
{
"log_manager",
"-l",
"LOGFILE_ERROR",
"-j",
path? path:TEST_LOG_DIR,
NULL
};
skygw_logmanager_init(argc,argv);
skygw_logmanager_init(logdir, argc, argv);
poll_init();
hkinit();
}

View File

@ -18,7 +18,7 @@ int harness_init(int argc, char** argv, HARNESS_INSTANCE** inst){
char** optstr;
if(!(argc == 2 && strcmp(argv[1],"-h") == 0)){
skygw_logmanager_init(0,NULL);
skygw_logmanager_init(NULL,0,NULL);
}
if(!(instance.head = calloc(1,sizeof(FILTERCHAIN))))
@ -52,12 +52,10 @@ int harness_init(int argc, char** argv, HARNESS_INSTANCE** inst){
getcwd(cwd,sizeof(cwd));
sprintf(tmp,"%s",cwd);
optstr = (char**)malloc(sizeof(char*)*4);
optstr = (char**)malloc(sizeof(char*)*2);
optstr[0] = strdup("log_manager");
optstr[1] = strdup("-j");
optstr[2] = strdup(tmp);
optstr[3] = NULL;
skygw_logmanager_init( 3, optstr);
optstr[1] = NULL;
skygw_logmanager_init(tmp, 1, optstr);
free(optstr);
rval = process_opts(argc,argv);

View File

@ -87,7 +87,7 @@ return 1;
int main(int argc, char **argv) {
char** arg_vector;
int arg_count = 4;
int arg_count = 1;
ROUTER_INSTANCE *inst;
int fd;
int ret;
@ -135,11 +135,8 @@ int main(int argc, char **argv) {
}
arg_vector[0] = "logmanager";
arg_vector[1] = "-j";
arg_vector[2] = "/tmp/maxbinlogcheck";
arg_vector[3] = "-o";
arg_vector[4] = NULL;
skygw_logmanager_init(arg_count,arg_vector);
arg_vector[1] = NULL;
skygw_logmanager_init(NULL, arg_count, arg_vector);
skygw_log_set_augmentation(0);

View File

@ -77,7 +77,7 @@ int main(int argc, char **argv) {
ROUTER_INSTANCE *inst;
int ret;
int rc;
int arg_count = 4;
int arg_count = 1;
char error_string[BINLOG_ERROR_MSG_LEN + 1] = "";
CHANGE_MASTER_OPTIONS change_master;
char query[255+1]="";
@ -100,11 +100,8 @@ int main(int argc, char **argv) {
}
arg_vector[0] = "logmanager";
arg_vector[1] = "-j";
arg_vector[2] = "/tmp/maxbinlogcheck";
arg_vector[3] = "-o";
arg_vector[4] = NULL;
skygw_logmanager_init(arg_count,arg_vector);
arg_vector[1] = NULL;
skygw_logmanager_init(NULL, arg_count,arg_vector);
free(arg_vector);
skygw_log_disable(LOGFILE_DEBUG);