From 1ab8420e91699eba1a291247af4936a124ca39c1 Mon Sep 17 00:00:00 2001 From: vraatikka Date: Thu, 8 Aug 2013 15:46:33 +0300 Subject: [PATCH 1/4] Added an example to user command : help show dbusers and removed erroneous complain about unknown subcommand if show dbusers is called without arguments. --- server/modules/routing/debugcmd.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/server/modules/routing/debugcmd.c b/server/modules/routing/debugcmd.c index cee02a32d..057809a35 100644 --- a/server/modules/routing/debugcmd.c +++ b/server/modules/routing/debugcmd.c @@ -85,7 +85,7 @@ struct subcommand showoptions[] = { {0, 0, 0} }, { "dcb", 1, dprintDCB, "Show a single descriptor control block e.g. show dcb 0x493340", {ARG_TYPE_ADDRESS, 0, 0} }, - { "dbusers", 1, dcb_usersPrint, "Show statistics and user names for a service's user table", + { "dbusers", 1, dcb_usersPrint, "Show statistics and user names for a service's user table.\n\t\tExample : show dbusers ", {ARG_TYPE_ADDRESS, 0, 0} }, { "epoll", 0, dprintPollStats, "Show the poll statistics", {0, 0, 0} }, @@ -251,8 +251,8 @@ convert_arg(char *arg, int arg_type) /** * We have a complete line from the user, lookup the commands and execute them * - * Commands are tokenised based on white space and then the firt - * word is checked againts the cmds table. If a amtch is found the + * Commands are tokenised based on white space and then the first + * word is checked againts the cmds table. If a match is found the * second word is compared to the different options for that command. * * Commands may also take up to 3 additional arguments, these are all @@ -332,6 +332,7 @@ unsigned long arg1, arg2, arg3; { for (j = 0; cmds[i].options[j].arg1; j++) { + found = 1; /**< command and sub-command match */ if (strcasecmp(args[1], cmds[i].options[j].arg1) == 0) { if (argc != cmds[i].options[j].n_args) @@ -384,14 +385,13 @@ unsigned long arg1, arg2, arg3; dcb_printf(dcb, "Invalid argument: %s\n", args[4]); } - found = 1; } } } if (!found) { dcb_printf(dcb, - "Unknown option for the %s command. Valid sub-commands are:\n", + "Unknown or missing option for the %s command. Valid sub-commands are:\n", cmds[i].cmd); for (j = 0; cmds[i].options[j].arg1; j++) { From 8a9d178785a458ba9aef2bb6e1966aa4ea778c9a Mon Sep 17 00:00:00 2001 From: Massimiliano Pinto Date: Thu, 8 Aug 2013 18:40:02 +0200 Subject: [PATCH 2/4] Fixed bug for invalid memory access in row[1]+1 when row[1] is "" --- server/core/dbusers.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/server/core/dbusers.c b/server/core/dbusers.c index 0067ff28e..48762483e 100644 --- a/server/core/dbusers.c +++ b/server/core/dbusers.c @@ -24,6 +24,7 @@ * * Date Who Description * 24/06/2013 Massimiliano Pinto Initial implementation + * 08/08/2013 Massimiliano Pinto Fixed bug for invalid memory access in row[1]+1 when row[1] is "" * * @endverbatim */ @@ -171,7 +172,7 @@ getUsers(SERVICE *service, struct users *users) while ((row = mysql_fetch_row(result))) { // we assume here two fields are returned !!! // now adding to the hastable user and passwd+1 (escaping the first byte that is '*') - users_add(users, row[0], row[1]+1); + users_add(users, row[0], strlen(row[1]) ? row[1]+1 : row[1]); total_users++; } From 0ad25ba3ab32d658ba015848a64d0f307c5fbf36 Mon Sep 17 00:00:00 2001 From: vraatikka Date: Thu, 8 Aug 2013 23:38:00 +0300 Subject: [PATCH 3/4] Added functions skygw_log_enable(logfile_id_t) and skygw_log_disable(logfile_id_t) to Log manager API. By calling them log writing to any of predefined log files can be switched on or off. Added simple test to testlog.c --- log_manager/log_manager.cc | 158 +++++++++++++++++++++++++++++++++++-- log_manager/log_manager.h | 10 +-- log_manager/test/testlog.c | 70 +++++++++++++++- utils/skygw_utils.cc | 29 +++++-- 4 files changed, 247 insertions(+), 20 deletions(-) diff --git a/log_manager/log_manager.cc b/log_manager/log_manager.cc index 62e9e0cf9..ad159afd2 100644 --- a/log_manager/log_manager.cc +++ b/log_manager/log_manager.cc @@ -87,13 +87,15 @@ typedef struct blockbuf_st { skygw_chk_t bb_chk_tail; } blockbuf_t; -/** logfile object corresponds to physical file(s) where +/** + * logfile object corresponds to physical file(s) where * certain log is written. */ struct logfile_st { skygw_chk_t lf_chk_top; flat_obj_state_t lf_state; bool lf_init_started; + bool lf_enabled; logmanager_t* lf_lmgr; /** fwr_logmes is for messages from log clients */ skygw_message_t* lf_logmes; @@ -130,6 +132,7 @@ struct fnames_conf_st { struct logmanager_st { skygw_chk_t lm_chk_top; bool lm_enabled; + int lm_enabled_logfiles; simple_mutex_t lm_mutex; size_t lm_nlinks; /** fwr_logmes is for messages from log clients */ @@ -190,6 +193,8 @@ static char* blockbuf_get_writepos( static void blockbuf_register(blockbuf_t* bb); static void blockbuf_unregister(blockbuf_t* bb); +static bool log_set_enabled(logfile_id_t id, bool val); + const char* get_suffix_default(void) { @@ -245,6 +250,8 @@ static bool logmanager_init_nomutex( lm->lm_chk_tail = CHK_NUM_LOGMANAGER; lm->lm_clientmes = skygw_message_init(); lm->lm_logmes = skygw_message_init(); + lm->lm_enabled_logfiles |= LOGFILE_ERROR; + lm->lm_enabled_logfiles |= LOGFILE_MESSAGE; fn = &lm->lm_fnames_conf; fw = &lm->lm_filewriter; fn->fn_state = UNINIT; @@ -276,6 +283,7 @@ static bool logmanager_init_nomutex( } /** Wait message from filewriter_thr */ skygw_message_wait(fw->fwr_clientmes); + succp = TRUE; lm->lm_enabled = TRUE; @@ -639,7 +647,6 @@ static char* blockbuf_get_writepos( mlist_node_t* node; blockbuf_t* bb; ss_debug(bool succp;) - ss_debug(int i=0;) CHK_LOGMANAGER(lm); @@ -841,6 +848,114 @@ static blockbuf_t* blockbuf_init( } +int skygw_log_enable( + logfile_id_t id) +{ + bool err = 0; + + if (!logmanager_register(TRUE)) { + fprintf(stderr, "ERROR: Can't register to logmanager\n"); + err = -1; + goto return_err; + } + CHK_LOGMANAGER(lm); + + if (log_set_enabled(id, TRUE)) { + lm->lm_enabled_logfiles |= id; + } + + logmanager_unregister(); +return_err: + return err; +} + + +int skygw_log_disable( + logfile_id_t id) +{ + bool err = 0; + + if (!logmanager_register(TRUE)) { + fprintf(stderr, "ERROR: Can't register to logmanager\n"); + err = -1; + goto return_err; + } + CHK_LOGMANAGER(lm); + + if (log_set_enabled(id, FALSE)) { + lm->lm_enabled_logfiles &= ~id; + } + + logmanager_unregister(); +return_err: + return err; +} + + +static bool log_set_enabled( + logfile_id_t id, + bool val) +{ + char* logstr; + va_list notused; + bool oldval; + bool succp = FALSE; + int err = 0; + logfile_t* lf; + + CHK_LOGMANAGER(lm); + + if (id < LOGFILE_FIRST || id > LOGFILE_LAST) { + char* errstr = "Invalid logfile id argument."; + /** + * invalid id, since we don't have logfile yet. + */ + err = logmanager_write_log(LOGFILE_ERROR, + TRUE, + FALSE, + strlen(errstr)+1, + errstr, + notused); + if (err != 0) { + fprintf(stderr, + "Writing to logfile %s failed.\n", + STRLOGID(LOGFILE_ERROR)); + } + ss_dassert(FALSE); + goto return_succp; + } + lf = &lm->lm_logfile[id]; + CHK_LOGFILE(lf); + + if (val) { + logstr = strdup("---\tLogging is enabled\t--"); + } else { + logstr = strdup("---\tLogging is disabled\t--"); + } + oldval = lf->lf_enabled; + lf->lf_enabled = val; + err = logmanager_write_log(id, + TRUE, + FALSE, + strlen(logstr)+1, + logstr, + notused); + free(logstr); + + if (err != 0) { + lf->lf_enabled = oldval; + fprintf(stderr, + "log_set_enabled failed. Writing notification to logfile %s " + "failed.\n ", + STRLOGID(id)); + goto return_succp; + } + succp = TRUE; +return_succp: + return succp; +} + + int skygw_log_write_flush( logfile_id_t id, char* str, @@ -862,6 +977,13 @@ int skygw_log_write_flush( STRLOGID(id), str); #endif + /** + * If particular log is disabled only unregister and return. + */ + if (!(lm->lm_enabled_logfiles & id)) { + err = 1; + goto return_unregister; + } /** * Find out the length of log string (to be formatted str). */ @@ -915,6 +1037,13 @@ int skygw_log_write( STRLOGID(id), str); #endif + /** + * If particular log is disabled only unregister and return. + */ + if (!(lm->lm_enabled_logfiles & id)) { + err = 1; + goto return_unregister; + } /** * Find out the length of log string (to be formatted str). */ @@ -1228,19 +1357,20 @@ static char* fname_conf_get_suffix( static bool logfiles_init( - logmanager_t* lmgr) + logmanager_t* lm) { bool succp = TRUE; int i = LOGFILE_FIRST; while(i<=LOGFILE_LAST && succp) { - succp = logfile_init(&lmgr->lm_logfile[i], (logfile_id_t)i, lmgr); - + succp = logfile_init(&lm->lm_logfile[i], (logfile_id_t)i, lm); + if (!succp) { fprintf(stderr, "Initializing logfiles failed\n"); break; } - i++; + i <<= 1; + } return succp; } @@ -1278,6 +1408,7 @@ static bool logfile_init( logfile->lf_lmgr = logmanager; logfile->lf_flushflag = FALSE; logfile->lf_spinlock = 0; + logfile->lf_enabled = logmanager->lm_enabled_logfiles & logfile_id; /** Read existing files to logfile->lf_files_list and create * new file for log named after / */ @@ -1397,6 +1528,7 @@ static bool filewriter_init( logfile_t* lf; logfile_id_t id; int i; + char* start_msg_str; CHK_LOGMANAGER(logmanager); @@ -1412,7 +1544,7 @@ static bool filewriter_init( if (fw->fwr_logmes == NULL || fw->fwr_clientmes == NULL) { goto return_succp; } - for (i=LOGFILE_FIRST; i<=LOGFILE_LAST; i++) { + for (i=LOGFILE_FIRST; i<=LOGFILE_LAST; i <<= 1) { id = (logfile_id_t)i; lf = logmanager_get_logfile(logmanager, id); fw->fwr_file[id] = skygw_file_init(lf->lf_full_name); @@ -1420,6 +1552,16 @@ static bool filewriter_init( if (fw->fwr_file[id] == NULL) { goto return_succp; } + if (lf->lf_enabled) { + start_msg_str = strdup("---\tLogging is enabled.\n"); + } else { + start_msg_str = strdup("---\tLogging is disabled.\n"); + } + skygw_file_write(fw->fwr_file[id], + (void *)start_msg_str, + strlen(start_msg_str), + TRUE); + free(start_msg_str); } fw->fwr_state = RUN; CHK_FILEWRITER(fw); @@ -1534,7 +1676,7 @@ static void* thr_filewriter_fun( flushall_logfiles = skygw_thread_must_exit(thr); /** Process all logfiles which have buffered writes. */ - for (i=LOGFILE_FIRST; i<=LOGFILE_LAST; i++) { + for (i=LOGFILE_FIRST; i<=LOGFILE_LAST; i <<= 1) { /** * Get file pointer of current logfile. */ diff --git a/log_manager/log_manager.h b/log_manager/log_manager.h index a15913b41..cfaa4f567 100644 --- a/log_manager/log_manager.h +++ b/log_manager/log_manager.h @@ -23,16 +23,15 @@ typedef struct fnames_conf_st fnames_conf_t; typedef struct logmanager_st logmanager_t; typedef enum { - LOGFILE_TRACE = 0, + LOGFILE_TRACE = 1, LOGFILE_FIRST = LOGFILE_TRACE, - LOGFILE_MESSAGE, - LOGFILE_ERROR, + LOGFILE_MESSAGE = 2, + LOGFILE_ERROR = 4, LOGFILE_LAST = LOGFILE_ERROR } logfile_id_t; typedef enum { FILEWRITER_INIT, FILEWRITER_RUN, FILEWRITER_DONE } filewriter_state_t; -typedef enum { LOGFILE_INIT, LOGFILE_OPENED, LOGFILE_DONE } logfile_state_t; /** * UNINIT means zeroed memory buffer allocated for the struct. @@ -56,7 +55,8 @@ void skygw_log_done(void); int skygw_log_write(logfile_id_t id, char* format, ...); int skygw_log_flush(logfile_id_t id); int skygw_log_write_flush(logfile_id_t id, char* format, ...); - +int skygw_log_enable(logfile_id_t id); +int skygw_log_disable(logfile_id_t id); EXTERN_C_BLOCK_END diff --git a/log_manager/test/testlog.c b/log_manager/test/testlog.c index fe3a0ce44..13300ec6b 100644 --- a/log_manager/test/testlog.c +++ b/log_manager/test/testlog.c @@ -44,6 +44,9 @@ static void* thr_run_morelog(void* data); #define TEST1 #define TEST2 #endif + +#define TEST3 + int main(int argc, char* argv[]) { int err = 0; @@ -175,7 +178,9 @@ int main(int argc, char* argv[]) free(thr[i]); } #endif + #if defined(TEST2) + fprintf(stderr, "\nStarting test #2 \n"); /** 2 */ @@ -229,7 +234,70 @@ int main(int argc, char* argv[]) /** Test ended here */ skygw_message_done(mes); simple_mutex_done(mtx); -#endif +#endif /* TEST 2 */ + +#if defined(TEST3) + +/** + * Test enable/disable log. + */ + r = skygw_logmanager_init(argc, argv); + ss_dassert(r); + + logstr = ("1.\tWrite to ERROR and MESSAGE logs."); + err = skygw_log_write(LOGFILE_MESSAGE, logstr); + ss_dassert(err == 0); + err = skygw_log_write(LOGFILE_TRACE, logstr); + ss_dassert(err != 0); /**< Must fail */ + err = skygw_log_write(LOGFILE_ERROR, logstr); + ss_dassert(err == 0); + + skygw_log_enable(LOGFILE_TRACE); + + logstr = ("2.\tWrite to ERROR and MESSAGE and TRACE logs."); + err = skygw_log_write(LOGFILE_MESSAGE, logstr); + ss_dassert(err == 0); + err = skygw_log_write(LOGFILE_TRACE, logstr); + ss_dassert(err == 0); + err = skygw_log_write(LOGFILE_ERROR, logstr); + ss_dassert(err == 0); + + skygw_log_disable(LOGFILE_ERROR); + + logstr = ("3.\tWrite to MESSAGE and TRACE logs."); + err = skygw_log_write(LOGFILE_MESSAGE, logstr); + ss_dassert(err == 0); + err = skygw_log_write(LOGFILE_TRACE, logstr); + ss_dassert(err == 0); + err = skygw_log_write(LOGFILE_ERROR, logstr); + ss_dassert(err != 0); /**< Must fail */ + + skygw_log_disable(LOGFILE_MESSAGE); + skygw_log_disable(LOGFILE_TRACE); + + logstr = ("4.\tWrite to none."); + err = skygw_log_write(LOGFILE_MESSAGE, logstr); + ss_dassert(err != 0); /**< Must fail */ + err = skygw_log_write(LOGFILE_TRACE, logstr); + ss_dassert(err != 0); /**< Must fail */ + err = skygw_log_write(LOGFILE_ERROR, logstr); + ss_dassert(err != 0); /**< Must fail */ + + skygw_log_enable(LOGFILE_ERROR); + skygw_log_enable(LOGFILE_MESSAGE); + + logstr = ("4.\tWrite to ERROR and MESSAGE logs."); + err = skygw_log_write(LOGFILE_MESSAGE, logstr); + ss_dassert(err == 0); + err = skygw_log_write(LOGFILE_TRACE, logstr); + ss_dassert(err != 0); /**< Must fail */ + err = skygw_log_write(LOGFILE_ERROR, logstr); + ss_dassert(err == 0); + + skygw_logmanager_done(); + +#endif /* TEST 3 */ + fprintf(stderr, ".. done.\n"); return err; } diff --git a/utils/skygw_utils.cc b/utils/skygw_utils.cc index 3c613b163..b654d958c 100644 --- a/utils/skygw_utils.cc +++ b/utils/skygw_utils.cc @@ -1503,12 +1503,15 @@ static bool file_write_header( size_t wbytes1; size_t wbytes2; size_t wbytes3; + size_t wbytes4; size_t len1; size_t len2; size_t len3; + size_t len4; const char* header_buf1; char* header_buf2 = NULL; - const char* header_buf3; + char* header_buf3 = NULL; + const char* header_buf4; time_t* t; struct tm* tm; @@ -1519,29 +1522,38 @@ static bool file_write_header( CHK_FILE(file); header_buf1 = "\n\nSkySQL MaxScale\t"; - header_buf2 = strdup(asctime(tm)); - header_buf3 = "------------------------------------------\n"; + header_buf2 = (char *)calloc(1, strlen(file->sf_fname)+2); + snprintf(header_buf2, strlen(file->sf_fname)+2, "%s ", file->sf_fname); + header_buf3 = strdup(asctime(tm)); + header_buf4 = "---------------------------------------------------------" + "---------------------------\n"; if (header_buf2 == NULL) { goto return_succp; } + if (header_buf3 == NULL) { + goto return_succp; + } + len1 = strlen(header_buf1); len2 = strlen(header_buf2); len3 = strlen(header_buf3); + len4 = strlen(header_buf4); #if defined(LAPTOP_TEST) usleep(DISKWRITE_LATENCY); #else wbytes1=fwrite((void*)header_buf1, len1, 1, file->sf_file); wbytes2=fwrite((void*)header_buf2, len2, 1, file->sf_file); wbytes3=fwrite((void*)header_buf3, len3, 1, file->sf_file); + wbytes4=fwrite((void*)header_buf4, len4, 1, file->sf_file); - if (wbytes1 != 1 || wbytes2 != 1 || wbytes3 != 1) { + if (wbytes1 != 1 || wbytes2 != 1 || wbytes3 != 1 || wbytes4 != 1) { fprintf(stderr, "Writing header %s %s %s to %s failed.\n", header_buf1, header_buf2, header_buf3, - file->sf_fname); + header_buf4); perror("Logfile header write.\n"); goto return_succp; } @@ -1550,7 +1562,12 @@ static bool file_write_header( succp = TRUE; return_succp: - free(header_buf2); + if (header_buf2 != NULL) { + free(header_buf2); + } + if (header_buf3 != NULL) { + free(header_buf3); + } free(t); free(tm); return succp; From 43fb6b87f19d204a80c76f9969ba0e289726ad36 Mon Sep 17 00:00:00 2001 From: vraatikka Date: Fri, 9 Aug 2013 10:01:34 +0300 Subject: [PATCH 4/4] Enabled trace log in DEBUG=Y builds. Modified testlog-c accordingly. --- log_manager/log_manager.cc | 3 +++ log_manager/test/testlog.c | 6 ++++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/log_manager/log_manager.cc b/log_manager/log_manager.cc index ad159afd2..9aac6f45e 100644 --- a/log_manager/log_manager.cc +++ b/log_manager/log_manager.cc @@ -252,6 +252,9 @@ static bool logmanager_init_nomutex( lm->lm_logmes = skygw_message_init(); lm->lm_enabled_logfiles |= LOGFILE_ERROR; lm->lm_enabled_logfiles |= LOGFILE_MESSAGE; +#if defined(SS_DEBUG) + lm->lm_enabled_logfiles |= LOGFILE_TRACE; +#endif fn = &lm->lm_fnames_conf; fw = &lm->lm_filewriter; fn->fn_state = UNINIT; diff --git a/log_manager/test/testlog.c b/log_manager/test/testlog.c index 13300ec6b..71813f861 100644 --- a/log_manager/test/testlog.c +++ b/log_manager/test/testlog.c @@ -238,12 +238,14 @@ int main(int argc, char* argv[]) #if defined(TEST3) -/** + /** * Test enable/disable log. */ r = skygw_logmanager_init(argc, argv); ss_dassert(r); - + + skygw_log_disable(LOGFILE_TRACE); + logstr = ("1.\tWrite to ERROR and MESSAGE logs."); err = skygw_log_write(LOGFILE_MESSAGE, logstr); ss_dassert(err == 0);