diff mbox

[LEDE-DEV,2/3,ubox] syslog: change log buffer size

Message ID 1464951549-4152-2-git-send-email-danutbug@gmail.com
State Changes Requested
Headers show

Commit Message

dbugnar June 3, 2016, 10:59 a.m. UTC
Change the log buffer size and copy the messages.
Copy the messages from the oldest to newest using the log_list.
First time we calculate the size of the all entries. The log_entries_size
calculates the size indifferent of the positions of the newest and oldest.

If we want to increase the log buffer size is simple, we just copy from the oldest
message to the newest all the messages into the new buffer.

In case we shrink the buffer we go over messages and decrease the entries size
with the size of each message until the entries size is less or equal to the new
size we allocate the new buffer.

Signed-off-by: Dan Bugnar <danutbug@gmail.com>
---
 log/syslog.c | 36 +++++++++++++++++++++++++++---------
 log/syslog.h |  2 +-
 2 files changed, 28 insertions(+), 10 deletions(-)

Comments

Conor O'Gorman June 3, 2016, 11:41 a.m. UTC | #1
On 03/06/16 11:59, Dan Bugnar wrote:
> Change the log buffer size and copy the messages.
> Copy the messages from the oldest to newest using the log_list.
> First time we calculate the size of the all entries. The log_entries_size
> calculates the size indifferent of the positions of the newest and oldest.
>
> If we want to increase the log buffer size is simple, we just copy from the oldest
> message to the newest all the messages into the new buffer.
>
> In case we shrink the buffer we go over messages and decrease the entries size
> with the size of each message until the entries size is less or equal to the new
> size we allocate the new buffer.
>
> Signed-off-by: Dan Bugnar <danutbug@gmail.com>
> ---
>   log/syslog.c | 36 +++++++++++++++++++++++++++---------
>   log/syslog.h |  2 +-
>   2 files changed, 28 insertions(+), 10 deletions(-)
>
> diff --git a/log/syslog.c b/log/syslog.c
> index 2042e93..e24c678 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;
> @@ -213,6 +213,15 @@ syslog_open(void)
>   	return 0;
>   }
>   
> +static inline int
> +log_entries_size()
> +{
> +	if (newest > oldest)
> +		return (newest - oldest);
> +	else
> +		return (log_end - oldest + newest - log);
> +}
> +
>   struct log_head*
>   log_list(int count, struct log_head *h)
>   {
> @@ -237,26 +246,38 @@ log_list(int count, struct log_head *h)
>   }
>   
>   int
> -log_buffer_init(int size)
> +log_buffer_reinit(int size)
>   {
> +	if (size <= 0)
> +		size = LOG_DEFAULT_SIZE;
> +	if (log_size == size)
> +		return 0;
> +
>   	struct log_head *_log = malloc(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)) {
> +	if (log) {
>   		struct log_head *start = _log;
>   		struct log_head *end = ((void*) _log) + size;
>   		struct log_head *l;
> +		int entries_size = log_entries_size();
>   
>   		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)];
> +			int copy_len = PAD(sizeof(struct log_head)) + l->size;
And here we see PAD jump around again.

> +			if (entries_size < size) {
> +				memcpy(start, l, copy_len);
> +				start = (struct log_head *) &start->data[l->size];
> +			} else{
> +				entries_size -= copy_len;
> +			}
I find this structure unpleasent, but it appears to work.
The code should eventually run in the first block, after possibly 
skipping entries via the second block?

Still a bit worried about lengths due to the PAD issue.

>   			l = log_list(0, l);
>   		}
>   		free(log);
> @@ -276,13 +297,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);
>
John Crispin June 3, 2016, 11:48 a.m. UTC | #2
On 03/06/2016 12:59, Dan Bugnar wrote:
> Change the log buffer size and copy the messages.
> Copy the messages from the oldest to newest using the log_list.
> First time we calculate the size of the all entries. The log_entries_size
> calculates the size indifferent of the positions of the newest and oldest.
> 
> If we want to increase the log buffer size is simple, we just copy from the oldest
> message to the newest all the messages into the new buffer.
> 
> In case we shrink the buffer we go over messages and decrease the entries size
> with the size of each message until the entries size is less or equal to the new
> size we allocate the new buffer.
> 
> Signed-off-by: Dan Bugnar <danutbug@gmail.com>
> ---
>  log/syslog.c | 36 +++++++++++++++++++++++++++---------
>  log/syslog.h |  2 +-
>  2 files changed, 28 insertions(+), 10 deletions(-)
> 
> diff --git a/log/syslog.c b/log/syslog.c
> index 2042e93..e24c678 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 dont get defined to 0

>  static struct log_head *log, *log_end, *oldest, *newest;
>  static int current_id = 0;
>  static regex_t pat_prio;
> @@ -213,6 +213,15 @@ syslog_open(void)
>  	return 0;
>  }
>  
> +static inline int 
> +log_entries_size()

void prototype ?

> +{
> +	if (newest > oldest)
> +		return (newest - oldest);
> +	else
> +		return (log_end - oldest + newest - log);
> +}
> +
>  struct log_head*
>  log_list(int count, struct log_head *h)
>  {
> @@ -237,26 +246,38 @@ log_list(int count, struct log_head *h)
>  }
>  
>  int
> -log_buffer_init(int size)
> +log_buffer_reinit(int size)
>  {
> +	if (size <= 0)
> +		size = LOG_DEFAULT_SIZE;
> +	if (log_size == size)
> +		return 0;
> +

code before variable declarations ? did this compile without warning ?

>  	struct log_head *_log = malloc(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)) {
> +	if (log) {
>  		struct log_head *start = _log;
>  		struct log_head *end = ((void*) _log) + size;
>  		struct log_head *l;
> +		int entries_size = log_entries_size();
>  
>  		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)];
> +			int copy_len = PAD(sizeof(struct log_head)) + l->size;
> +			if (entries_size < size) {
> +				memcpy(start, l, copy_len);
> +				start = (struct log_head *) &start->data[l->size];
> +			} else{
> +				entries_size -= copy_len;
> +			}
>  			l = log_list(0, l);
>  		}
>  		free(log);
> @@ -276,13 +297,10 @@ log_buffer_init(int size)
>  void
>  log_init(int _log_size)
>  {
> -	if (_log_size > 0)
> -		log_size = _log_size;

what happened to log_size ? if you dont use it anymore remove it fully
please

> -
you are removing a blank like here


>  	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);
>  
>
dbugnar June 7, 2016, 7:05 a.m. UTC | #3
>                        if (entries_size < size) {
>                                memcpy(start, l, copy_len);
>                                start = (struct log_head *) &start->data[l->size];
>                        } else{
>                                entries_size -= copy_len;
>                        }
>

Yes, the first block is executed just when the entries_size it fits in
the new buffer size, otherwise we skip entries in the second block.

>   void
>   log_init(int _log_size)
>   {
>        if (_log_size > 0)
>                log_size = _log_size;
>
> what happened to log_size ? if you dont use it anymore remove it fully
> please
>
log_size is used when the new buffer size is equal to the current
buffer size and we keep the current buffer intact.
diff mbox

Patch

diff --git a/log/syslog.c b/log/syslog.c
index 2042e93..e24c678 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;
@@ -213,6 +213,15 @@  syslog_open(void)
 	return 0;
 }
 
+static inline int 
+log_entries_size()
+{
+	if (newest > oldest)
+		return (newest - oldest);
+	else
+		return (log_end - oldest + newest - log);
+}
+
 struct log_head*
 log_list(int count, struct log_head *h)
 {
@@ -237,26 +246,38 @@  log_list(int count, struct log_head *h)
 }
 
 int
-log_buffer_init(int size)
+log_buffer_reinit(int size)
 {
+	if (size <= 0)
+		size = LOG_DEFAULT_SIZE;
+	if (log_size == size)
+		return 0;
+
 	struct log_head *_log = malloc(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)) {
+	if (log) {
 		struct log_head *start = _log;
 		struct log_head *end = ((void*) _log) + size;
 		struct log_head *l;
+		int entries_size = log_entries_size();
 
 		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)];
+			int copy_len = PAD(sizeof(struct log_head)) + l->size;
+			if (entries_size < size) {
+				memcpy(start, l, copy_len);
+				start = (struct log_head *) &start->data[l->size];
+			} else{
+				entries_size -= copy_len;
+			}
 			l = log_list(0, l);
 		}
 		free(log);
@@ -276,13 +297,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);