Hi David, Sorry for the delay, but each time I looked at it I started to scratch my head wondering about the real purpose of the change and/or the reason for the current code currently being that way.
On Thu, Feb 18, 2016 at 09:58:49PM +0000, David CARLIER wrote: > HI all, > > There is a tiny patch proposal for the log component. >From f1924d813f37cd6444d9251605fdd823efb7fe20 Mon Sep 17 00:00:00 2001 From: David Carlier <[email protected]> Date: Thu, 18 Feb 2016 21:46:48 +0000 Subject: [PATCH] CLEANUP: log The string to be escaped is not marked at const, and it would be safer to copy instead of casting. --- src/log.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/log.c b/src/log.c index 3e25bc7..94864e1 100644 --- a/src/log.c +++ b/src/log.c @@ -831,13 +831,19 @@ char *lf_text_len(char *dst, const char *src, size_t len, size_t size, struct lo if (src && len) { if (node->options & LOG_OPT_ESC) { struct chunk chunk; - char *ret; + char *ret, *ssrc; - chunk_initlen(&chunk, (char *)src, 0, len); + ssrc = my_strndup(src, len); + if (ssrc == NULL) + return NULL; + chunk_initlen(&chunk, ssrc, 0, len); ret = escape_chunk(dst, dst + size, '\\', rfc5424_escape_map, &chunk); - if (ret == NULL || *ret != '\0') + if (ret == NULL || *ret != '\0') { + chunk_destroy(&chunk); return NULL; + } len = ret - dst; + chunk_destroy(&chunk); } else { if (++len > size) We cannot afford to call strndup() for each log element to be escaped, it adds too much overhead, it performs a malloc()! I'd suggest two approaches (I haven't looked at the calling places) : - either you simply use another chunk as a temporary holder for that part ; - or we check if it's acceptable to change the const char* for a char * in the argument and expect the caller to have a R/W buffer itself. Hmmm just thinking about it, it looks like we don't modify src anyway here, so it's even more overkill to strdup() something we're not going to change. Thus I'd rather go for something much cleaner : let's add a new function "chunk_initlen_const()" to chunk.h taking a const char* and a length, and performing the cast itself, as well as leaving the size to zero (so that we can't modify the contents). Then you just have to make use of it in the function above and the problem is gone. It's possible that other cases exist as well by the way. Best regards, Willy
