[LEDE-DEV] [PATCH 1/2] [ubox] syslog: use realloc to change log buffer size

[Alexandru Ardelean]

On Thu, May 12, 2016 at 3:09 PM, Dan Bugnar <danutbug at gmail.com> wrote:
> From: Dan Bugnar <danutbug at gmail.com>
>
> Change the log buffer size and copy the messages.
>
> Signed-off-by: Dan Bugnar <danutbug at gmail.com>
> ---
>  log/syslog.c | 37 +++++++++++++++----------------------
>  log/syslog.h |  2 +-
>  2 files changed, 16 insertions(+), 23 deletions(-)
>
> diff --git a/log/syslog.c b/log/syslog.c
> index e8b6774..d6cb868 100644
> --- a/log/syslog.c
> +++ b/log/syslog.c
> @@ -42,7 +42,7 @@
>  #define PAD(x) (x % 4) ? (((x) - (x % 4)) + 4) : (x)
>
>  static char *log_dev = LOG_DEFAULT_SOCKET;
> -static int log_size = LOG_DEFAULT_SIZE;
> +static int log_size = 0;
>  static struct log_head *log, *log_end, *oldest, *newest;
>  static int current_id = 0;
>  static regex_t pat_prio;
> @@ -237,34 +237,30 @@ log_list(int count, struct log_head *h)
>  }
>
>  int
> -log_buffer_init(int size)
> +log_buffer_reinit(int size)
>  {
> -       struct log_head *_log = malloc(size);
> +       if (size <= 0)
> +               size = LOG_DEFAULT_SIZE;
> +       if (log_size == size)
> +               return 0;
> +
> +       struct log_head *_log = realloc(log, size);
>
>         if (!_log) {
> +               oldest = newest = log = NULL;
>                 fprintf(stderr, "Failed to initialize log buffer with size %d\n", log_size);
>                 return -1;
>         }
>
> -       memset(_log, 0, size);
> -
>         if (log && ((log_size + sizeof(struct log_head)) < size)) {
> -               struct log_head *start = _log;
> -               struct log_head *end = ((void*) _log) + size;
> -               struct log_head *l;
> -
> -               l = log_list(0, NULL);
> -               while ((start < end) && l && l->size) {
> -                       memcpy(start, l, PAD(sizeof(struct log_head) + l->size));
> -                       start = (struct log_head *) &l->data[PAD(l->size)];
> -                       l = log_list(0, l);
> -               }
> -               free(log);
> -               newest = start;
> +               newest = (_log + (newest - log));
>                 newest->size = 0;
> -               oldest = log = _log;
> +               memset(newest, 0, size - log_size);
> +               oldest = (_log + (oldest - log));
> +               log = _log;
>                 log_end = ((void*) log) + size;
>         } else {
> +               memset(_log, 0, size);
>                 oldest = newest = log = _log;
>                 log_end = ((void*) log) + size;
>         }
> @@ -276,13 +272,10 @@ log_buffer_init(int size)
>  void
>  log_init(int _log_size)
>  {
> -       if (_log_size > 0)
> -               log_size = _log_size;
> -
>         regcomp(&pat_prio, "^<([0-9]*)>(.*)", REG_EXTENDED);
>         regcomp(&pat_tstamp, "^\[[ 0]*([0-9]*).([0-9]*)] (.*)", REG_EXTENDED);
>
> -       if (log_buffer_init(log_size)) {
> +       if (log_buffer_reinit(_log_size)) {
>                 fprintf(stderr, "Failed to allocate log memory\n");
>                 exit(-1);
>         }
> diff --git a/log/syslog.h b/log/syslog.h
> index 81a039f..ed5a41b 100644
> --- a/log/syslog.h
> +++ b/log/syslog.h
> @@ -35,7 +35,7 @@ void log_shutdown(void);
>
>  typedef void (*log_list_cb)(struct log_head *h);
>  struct log_head* log_list(int count, struct log_head *h);
> -int log_buffer_init(int size);
> +int log_buffer_reinit(int size);
>  void log_add(char *buf, int size, int source);
>  void ubus_notify_log(struct log_head *l);
>
> --
> 2.8.1
>
>
> _______________________________________________
> Lede-dev mailing list
> Lede-dev at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/lede-dev

So, then, let's move the discussion to this patch, about dynamically
increasing the log size.
For reference, this is the initial code (that tried to do dynamic log
size increase) and the change that actually does it.

Like I said on the other thread.
There are 2 approaches on this initial (malloc() + memcpy() code):
1) make it work
2) remove it

The issue with this code, is that it allocs a new buffer (for the
log), but looks like it tries to copy over the initial log buffer.
And I think the stop condition (of the while loop) is not completely
correct. I think it tries to also write after/outside the initial
buffer somewhere.