Currently, we copy the key-name to a buffer, iterate over it to replace the full-stops with underscores, using `strchr` from the start of the buffer on each iteration, then append the buffer to the SQL statement. Apart from the inefficiency, `strncpy` is used to do the copies, which leads gcc to complain: ../../util/db.c:118:25: warning: `strncpy` output may be truncated copying 31 bytes from a string of length 31 Furthermore, the buffer is one character too short and so there is the possibility of overruns. Instead, append the key-name directly to the statement using `sprintf`, and run `strchr` from the last underscore on each iteration. Signed-off-by: Jeremy Sowden <jeremy@xxxxxxxxxx> --- util/db.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/util/db.c b/util/db.c index 2dbe0db2fbfe..339e39ef4797 100644 --- a/util/db.c +++ b/util/db.c @@ -96,8 +96,6 @@ static int sql_createstmt(struct ulogd_pluginstance *upi) (procedure[strlen("INSERT")] == '\0' || procedure[strlen("INSERT")] == ' ')) { char *stmt_val = mi->stmt; - char buf[ULOGD_MAX_KEYLEN]; - char *underscore; if(procedure[6] == '\0') { /* procedure == "INSERT" */ @@ -112,13 +110,18 @@ static int sql_createstmt(struct ulogd_pluginstance *upi) stmt_val += sprintf(stmt_val, "%s (", procedure); for (i = 0; i < upi->input.num_keys; i++) { + char *underscore; + if (upi->input.keys[i].flags & ULOGD_KEYF_INACTIVE) continue; - strncpy(buf, upi->input.keys[i].name, ULOGD_MAX_KEYLEN); - while ((underscore = strchr(buf, '.'))) + underscore = stmt_val; + + stmt_val += sprintf(stmt_val, "%s,", + upi->input.keys[i].name); + + while ((underscore = strchr(underscore, '.'))) *underscore = '_'; - stmt_val += sprintf(stmt_val, "%s,", buf); } *(stmt_val - 1) = ')'; -- 2.33.0