patterncMinor
Small library for logging to MySQL
Viewed 0 times
loggingmysqlsmallforlibrary
Problem
I've thrown together a utility for my programs to log to a database, but the code looks like a mess and I don't know how to clean it up. This library is going to be used from all of my other programs and it is therefore important to keep the code clean and secure.
I'm trying to achieve a fast and simple way to process my log entries, but I'm not really happy with the code structure in general. Any suggestions for improvements would be much appreciated.
Here is the table structure of
``
#include
#include
#include
#include
#include
#define STR_BUFFER_SIZE 256
#define NELEMS(x) (sizeof(x) / sizeof(x[0]))
void finish_with_error (MYSQL *);
int ltdb (LOG_ENTRY *ent)
{
MYSQL *con = NULL;
char time[16], date[16];
char query[STR_BUFFER_SIZE];
int len;
con = mysql_init(NULL);
if(NULL == con){
fprintf(stderr, "%s\n", mysql_error(con));
return 1;
}
if(NULL == mysql_real_connect(con, "localhost", "user", "password", "database", 0, NULL, 0)){
finish_with_error(con);
return 1;
}
strftime(time, NELEMS(time), "%H:%M:%S", ent->tm_info);
strftime(date, NELEMS(date), "%Y-%m-%d", ent->tm_info);
len = snprintf(query, STR_BUFFER_SIZE,
"INSERT INTO `logg` ("
" severity,event,source,time,date"
") VALUES ("
" '%d', '%s', '%s', '%s', '%s'"
")", ent->severity,
ent->event,
ent->source,
time,
date);
if(len >= STR_BUFFER_SIZE){
fprintf(stderr, "MySQL query too long! (> %d)\n", STR_BUFFER_SIZE);
return 1;
}
if(mysql_query(con, query)){
finish_with_error(con);
return 1;
}
mysql_close(con);
return 0;
}
void finish_with_error(MYSQL *con)
{
fprintf(stderr, "%s\n", mysql_error(con));
mysql_close(con);
}I'm trying to achieve a fast and simple way to process my log entries, but I'm not really happy with the code structure in general. Any suggestions for improvements would be much appreciated.
Here is the table structure of
logg``
CREATE TABLE IF NOT EXISTS logg (
id int(10) unsigned NOT NULL AUTO_INCREMENT,
severity tinyint(4) NOT NULL,
event` varchar(128) NOT Solution
Assuming that
The 256-byte limit for the query buffer seems artificially limiting. I would expect your library to do whatever it takes to compose and execute the right SQL query without bungling it with an undersized buffer. One way to do that without
The buffer would need to be large enough to accommodate the fixed text and the substituted strings, keeping in mind that
Requiring the program to be recompiled to change the password is not proper. The caller should have to make a call to initialize the library with the database connection parameters. Another approach, which is still sloppy but slightly better, is for your library to read the connection parameters from a configuration file.
LOG_ENTRY.event can have arbitrary string content, your code is vulnerable to an SQL injection attack. You must escape strings in queries, and not copy them blindly into the query.The 256-byte limit for the query buffer seems artificially limiting. I would expect your library to do whatever it takes to compose and execute the right SQL query without bungling it with an undersized buffer. One way to do that without
malloc() or variable-length arrays is to define the struct to match the table definition.typedef struct {
int severity;
char event[129]; /* 128 characters plus NUL. Latin1 means 1 byte per character */
char source[33];
struct tm tm_info;
} LogEntry;The buffer would need to be large enough to accommodate the fixed text and the substituted strings, keeping in mind that
mysql_real_escape_string() can double the number of bytes. (I would use sizeof(FIXED_QUERY) + 2 sizeof(LogEntry) + 3 sizeof(int) + 20 as a generous estimate of the needed buffer size.) Out of paranoia for unterminated strings, I'd use placeholders like "%.128s" for snprintf().ltdb is such a cryptic function name. log_to_database() would be much better.Requiring the program to be recompiled to change the password is not proper. The caller should have to make a call to initialize the library with the database connection parameters. Another approach, which is still sloppy but slightly better, is for your library to read the connection parameters from a configuration file.
Code Snippets
typedef struct {
int severity;
char event[129]; /* 128 characters plus NUL. Latin1 means 1 byte per character */
char source[33];
struct tm tm_info;
} LogEntry;Context
StackExchange Code Review Q#80210, answer score: 3
Revisions (0)
No revisions yet.