From ce4f4fb3fa12bbec718da980c9cf9e937c2d3f88 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Helleu?= Date: Mon, 21 Aug 2023 16:01:59 +0200 Subject: [PATCH] logger: remove trailing empty line in display of backlog (closes #2002) --- ChangeLog.adoc | 1 + src/plugins/logger/logger-backlog.c | 52 ++-- src/plugins/logger/logger-tail.c | 124 +++++---- .../plugins/logger/test-logger-backlog.cpp | 12 +- .../unit/plugins/logger/test-logger-tail.cpp | 240 +++++++++++++++++- 5 files changed, 334 insertions(+), 95 deletions(-) diff --git a/ChangeLog.adoc b/ChangeLog.adoc index 58fca80d9..a39d7c3cb 100644 --- a/ChangeLog.adoc +++ b/ChangeLog.adoc @@ -47,6 +47,7 @@ Bug fixes:: * irc: add missing tags on multiline messages (issue #1987) * irc: fix redirection of command `/list` when the reply doesn't start with message 321 (start of /list) * irc: fix wrong time displayed for CTCP messages received from self nick (issue #2000) + * logger: remove trailing empty line in display of backlog (issue #2002) * perl: fix display of non-ASCII chars after load of a script with Perl >= 5.38 (issue #1996) * relay: synchronize nick modes with IRC client upon connection (issue #1984) * script: fix cursor position after `/script list -i` or `/script list -il` diff --git a/src/plugins/logger/logger-backlog.c b/src/plugins/logger/logger-backlog.c index 3c9f8dd6a..5be8e3769 100644 --- a/src/plugins/logger/logger-backlog.c +++ b/src/plugins/logger/logger-backlog.c @@ -168,7 +168,7 @@ struct t_arraylist * logger_backlog_group_messages (struct t_arraylist *lines) { int i, size, time_found; - char **message, **old_message, *str_date, *error; + char *message, *new_message, *str_date, *error; const char *ptr_line, *pos_message; struct tm tm_line; struct t_arraylist *messages; @@ -176,27 +176,37 @@ logger_backlog_group_messages (struct t_arraylist *lines) if (!lines) return NULL; + message = NULL; + size = weechat_arraylist_size (lines); messages = weechat_arraylist_new (size, 0, 1, &logger_backlog_msg_cmp_cb, NULL, &logger_backlog_msg_free_cb, NULL); if (!messages) - return NULL; - - message = weechat_string_dyn_alloc (256); - old_message = weechat_string_dyn_alloc (256); + goto error; for (i = size - 1; i >= 0; i--) { ptr_line = (const char *)weechat_arraylist_get (lines, i); - weechat_string_dyn_copy (old_message, *message); - weechat_string_dyn_copy (message, ptr_line); - if ((*old_message)[0]) + if (message) { - weechat_string_dyn_concat (message, "\n", -1); - weechat_string_dyn_concat (message, *old_message, -1); + new_message = malloc (strlen (ptr_line) + 1 + strlen (message) + 1); + if (!new_message) + goto error; + strcpy (new_message, ptr_line); + strcat (new_message, "\n"); + strcat (new_message, message); + free (message); + message = new_message; + } + else + { + message = malloc (strlen (ptr_line) + 1); + if (!message) + goto error; + strcpy (message, ptr_line); } time_found = 0; @@ -218,18 +228,26 @@ logger_backlog_group_messages (struct t_arraylist *lines) } if (time_found) { - weechat_arraylist_insert (messages, 0, strdup (*message)); - weechat_string_dyn_copy (message, NULL); + /* add message (will be freed when arraylist is destroyed) */ + weechat_arraylist_insert (messages, 0, message); + message = NULL; } } - if ((*message)[0]) - weechat_arraylist_insert (messages, 0, strdup (*message)); - - weechat_string_dyn_free (message, 1); - weechat_string_dyn_free (old_message, 1); + if (message) + { + /* add message (will be freed when arraylist is destroyed) */ + weechat_arraylist_insert (messages, 0, message); + } return messages; + +error: + if (message) + free (message); + if (messages) + weechat_arraylist_free (messages); + return NULL; } /* diff --git a/src/plugins/logger/logger-tail.c b/src/plugins/logger/logger-tail.c index e7d332720..21eb03778 100644 --- a/src/plugins/logger/logger-tail.c +++ b/src/plugins/logger/logger-tail.c @@ -100,7 +100,7 @@ logger_tail_lines_free_cb (void *data, struct t_arraylist *arraylist, struct t_arraylist * logger_tail_file (const char *filename, int lines) { - int fd; + int fd, count_read; off_t file_length, file_pos; size_t to_read; ssize_t bytes_read; @@ -108,21 +108,30 @@ logger_tail_file (const char *filename, int lines) char *ptr_buf, *pos_eol, *part_of_line, *new_part_of_line, *line; struct t_arraylist *list_lines; - if (!filename || (lines < 1)) + if (!filename || !filename[0] || (lines < 1)) return NULL; + fd = -1; + part_of_line = 0; + list_lines = NULL; + + /* allocate arraylist */ + list_lines = weechat_arraylist_new (lines, 0, 1, + &logger_tail_lines_cmp_cb, NULL, + &logger_tail_lines_free_cb, NULL); + if (!list_lines) + goto error; + /* open file */ fd = open (filename, O_RDONLY); if (fd == -1) - return NULL; + goto error; /* seek to the end of file */ + count_read = 0; file_length = lseek (fd, (off_t)0, SEEK_END); if (file_length <= 0) - { - close (fd); - return NULL; - } + goto error; to_read = file_length; file_pos = file_length - LOGGER_TAIL_BUFSIZE; if (file_pos < 0) @@ -132,71 +141,51 @@ logger_tail_file (const char *filename, int lines) lseek (fd, file_pos, SEEK_SET); /* loop until we have "lines" lines in list */ - part_of_line = NULL; - list_lines = weechat_arraylist_new (lines, 0, 1, - &logger_tail_lines_cmp_cb, NULL, - &logger_tail_lines_free_cb, NULL); while (lines > 0) { lseek (fd, file_pos, SEEK_SET); bytes_read = read (fd, buf, to_read); if (bytes_read <= 0) - { - if (part_of_line) - free (part_of_line); - weechat_arraylist_free (list_lines); - close (fd); - return NULL; - } + goto error; + count_read++; buf[bytes_read] = '\0'; + if ((count_read == 1) + && ((buf[bytes_read - 1] == '\n') || (buf[bytes_read - 1] == '\r'))) + { + /* ignore last new line of the file (on first block read only) */ + buf[bytes_read - 1] = '\0'; + bytes_read--; + } ptr_buf = buf + bytes_read - 1; while (ptr_buf && (ptr_buf >= buf)) { pos_eol = (char *)logger_tail_last_eol (buf, ptr_buf); - if ((pos_eol && (pos_eol[1] || part_of_line)) || (file_pos == 0)) + if (pos_eol) { - /* use data and part_of_line (if existing) to build a new line */ - if (!pos_eol) + ptr_buf = pos_eol - 1; + pos_eol[0] = '\0'; + pos_eol++; + if (part_of_line) { - ptr_buf = NULL; - pos_eol = buf; + line = malloc ((strlen (pos_eol) + + strlen (part_of_line) + 1)); + if (!line) + goto error; + strcpy (line, pos_eol); + strcat (line, part_of_line); + free (part_of_line); + part_of_line = NULL; + weechat_arraylist_insert (list_lines, 0, line); } else { - ptr_buf = pos_eol - 1; - pos_eol[0] = '\0'; - pos_eol++; - } - if (part_of_line || pos_eol[0]) - { - if (part_of_line) - { - line = malloc ((strlen (pos_eol) + - strlen (part_of_line) + 1)); - if (!line) - { - free (part_of_line); - weechat_arraylist_free (list_lines); - close (fd); - return NULL; - } - strcpy (line, pos_eol); - strcat (line, part_of_line); - free (part_of_line); - part_of_line = NULL; - weechat_arraylist_insert (list_lines, 0, line); - } - else - { - weechat_arraylist_insert (list_lines, 0, - strdup (pos_eol)); - } - lines--; - if (lines <= 0) - break; + weechat_arraylist_insert (list_lines, 0, strdup (pos_eol)); } + lines--; + if (lines <= 0) + break; } - else if (!pos_eol) + else { /* * beginning of read buffer reached without EOL, then we @@ -206,12 +195,7 @@ logger_tail_file (const char *filename, int lines) { new_part_of_line = malloc (strlen (buf) + strlen (part_of_line) + 1); if (!new_part_of_line) - { - free (part_of_line); - weechat_arraylist_free (list_lines); - close (fd); - return NULL; - } + goto error; strcpy (new_part_of_line, buf); strcat (new_part_of_line, part_of_line); free (part_of_line); @@ -220,12 +204,12 @@ logger_tail_file (const char *filename, int lines) else { part_of_line = malloc (strlen (buf) + 1); + if (!part_of_line) + goto error; strcpy (part_of_line, buf); } ptr_buf = NULL; } - else - ptr_buf = pos_eol - 1; } if (file_pos == 0) break; @@ -238,9 +222,21 @@ logger_tail_file (const char *filename, int lines) } if (part_of_line) - free (part_of_line); + { + /* add part of line (will be freed when arraylist is destroyed) */ + weechat_arraylist_insert (list_lines, 0, part_of_line); + } close (fd); return list_lines; + +error: + if (part_of_line) + free (part_of_line); + if (list_lines) + weechat_arraylist_free (list_lines); + if (fd >= 0) + close (fd); + return NULL; } diff --git a/tests/unit/plugins/logger/test-logger-backlog.cpp b/tests/unit/plugins/logger/test-logger-backlog.cpp index 7b968c204..189862d66 100644 --- a/tests/unit/plugins/logger/test-logger-backlog.cpp +++ b/tests/unit/plugins/logger/test-logger-backlog.cpp @@ -199,7 +199,10 @@ TEST(LoggerBacklog, GroupMessages) "end of line", "2023-06-04 21:15:34\t\tFirst line", "of multiline message", + "", "end of message", + "2023-06-04 21:15:37\t\tTwo lines with empty line", + "", "2023-06-04 21:15:40\t\tMessage on one line", NULL, }; @@ -207,7 +210,7 @@ TEST(LoggerBacklog, GroupMessages) POINTERS_EQUAL(NULL, logger_backlog_group_messages (NULL)); - lines = arraylist_new (3, 0, 1, + lines = arraylist_new (32, 0, 1, &test_logger_backlog_msg_cmp_cb, NULL, &test_logger_backlog_msg_free_cb, NULL); @@ -234,15 +237,18 @@ TEST(LoggerBacklog, GroupMessages) messages = logger_backlog_group_messages (lines); CHECK(messages); - LONGS_EQUAL(3, arraylist_size (messages)); + LONGS_EQUAL(4, arraylist_size (messages)); STRCMP_EQUAL("end of line", (const char *)arraylist_get (messages, 0)); STRCMP_EQUAL("2023-06-04 21:15:34\t\tFirst line\n" "of multiline message\n" + "\n" "end of message", (const char *)arraylist_get (messages, 1)); - STRCMP_EQUAL("2023-06-04 21:15:40\t\tMessage on one line", + STRCMP_EQUAL("2023-06-04 21:15:37\t\tTwo lines with empty line\n", (const char *)arraylist_get (messages, 2)); + STRCMP_EQUAL("2023-06-04 21:15:40\t\tMessage on one line", + (const char *)arraylist_get (messages, 3)); arraylist_free (messages); diff --git a/tests/unit/plugins/logger/test-logger-tail.cpp b/tests/unit/plugins/logger/test-logger-tail.cpp index 614cd1cea..6e23ea6a2 100644 --- a/tests/unit/plugins/logger/test-logger-tail.cpp +++ b/tests/unit/plugins/logger/test-logger-tail.cpp @@ -79,21 +79,24 @@ TEST(LoggerTail, LoggerTailLastEol) TEST(LoggerTail, LoggerTailFile) { - char *filename; - FILE *file; - const char *content = "line 1\nline 2\nline 3"; + const char *content_3_lines = "line 1\nline 2\nline 3\n"; + const char *content_5_lines = "line 1\nline 2\n\nline 3\n\n"; + char *filename, line[4096]; struct t_arraylist *lines; - - /* write a test file */ - filename = string_eval_path_home ("${weechat_data_dir}/test_file.txt", - NULL, NULL, NULL); - file = fopen (filename, "w"); - fwrite (content, 1, strlen (content), file); - fflush (file); - fclose (file); + FILE *file; + int i; POINTERS_EQUAL(NULL, logger_tail_file (NULL, 0)); POINTERS_EQUAL(NULL, logger_tail_file (NULL, 1)); + + /* write a small test file */ + filename = string_eval_path_home ("${weechat_data_dir}/test_file.txt", + NULL, NULL, NULL); + file = fopen (filename, "w"); + fwrite (content_3_lines, 1, strlen (content_3_lines), file); + fflush (file); + fclose (file); + POINTERS_EQUAL(NULL, logger_tail_file (filename, 0)); lines = logger_tail_file (filename, 1); @@ -127,4 +130,219 @@ TEST(LoggerTail, LoggerTailFile) unlink (filename); free (filename); + + /* write a small test file, with empty lines */ + filename = string_eval_path_home ("${weechat_data_dir}/test_file.txt", + NULL, NULL, NULL); + file = fopen (filename, "w"); + fwrite (content_5_lines, 1, strlen (content_5_lines), file); + fflush (file); + fclose (file); + + POINTERS_EQUAL(NULL, logger_tail_file (filename, 0)); + + lines = logger_tail_file (filename, 1); + CHECK(lines); + LONGS_EQUAL(1, arraylist_size (lines)); + STRCMP_EQUAL("", (const char *)arraylist_get (lines, 0)); + arraylist_free (lines); + + lines = logger_tail_file (filename, 2); + CHECK(lines); + LONGS_EQUAL(2, arraylist_size (lines)); + STRCMP_EQUAL("line 3", (const char *)arraylist_get (lines, 0)); + STRCMP_EQUAL("", (const char *)arraylist_get (lines, 1)); + arraylist_free (lines); + + lines = logger_tail_file (filename, 3); + CHECK(lines); + LONGS_EQUAL(3, arraylist_size (lines)); + STRCMP_EQUAL("", (const char *)arraylist_get (lines, 0)); + STRCMP_EQUAL("line 3", (const char *)arraylist_get (lines, 1)); + STRCMP_EQUAL("", (const char *)arraylist_get (lines, 2)); + arraylist_free (lines); + + lines = logger_tail_file (filename, 4); + CHECK(lines); + LONGS_EQUAL(4, arraylist_size (lines)); + STRCMP_EQUAL("line 2", (const char *)arraylist_get (lines, 0)); + STRCMP_EQUAL("", (const char *)arraylist_get (lines, 1)); + STRCMP_EQUAL("line 3", (const char *)arraylist_get (lines, 2)); + STRCMP_EQUAL("", (const char *)arraylist_get (lines, 3)); + arraylist_free (lines); + + lines = logger_tail_file (filename, 5); + CHECK(lines); + LONGS_EQUAL(5, arraylist_size (lines)); + STRCMP_EQUAL("line 1", (const char *)arraylist_get (lines, 0)); + STRCMP_EQUAL("line 2", (const char *)arraylist_get (lines, 1)); + STRCMP_EQUAL("", (const char *)arraylist_get (lines, 2)); + STRCMP_EQUAL("line 3", (const char *)arraylist_get (lines, 3)); + STRCMP_EQUAL("", (const char *)arraylist_get (lines, 4)); + arraylist_free (lines); + + lines = logger_tail_file (filename, 6); + CHECK(lines); + LONGS_EQUAL(5, arraylist_size (lines)); + STRCMP_EQUAL("line 1", (const char *)arraylist_get (lines, 0)); + STRCMP_EQUAL("line 2", (const char *)arraylist_get (lines, 1)); + STRCMP_EQUAL("", (const char *)arraylist_get (lines, 2)); + STRCMP_EQUAL("line 3", (const char *)arraylist_get (lines, 3)); + STRCMP_EQUAL("", (const char *)arraylist_get (lines, 4)); + arraylist_free (lines); + + unlink (filename); + free (filename); + + /* write a bigger test file */ + filename = string_eval_path_home ("${weechat_data_dir}/test_file.txt", + NULL, NULL, NULL); + file = fopen (filename, "w"); + for (i = 0; i < 1000; i++) + { + snprintf (line, sizeof (line), "this is a test, line %d\n", i + 1); + fwrite (line, 1, strlen (line), file); + } + fflush (file); + fclose (file); + + POINTERS_EQUAL(NULL, logger_tail_file (filename, 0)); + + lines = logger_tail_file (filename, 1); + CHECK(lines); + LONGS_EQUAL(1, arraylist_size (lines)); + STRCMP_EQUAL("this is a test, line 1000", (const char *)arraylist_get (lines, 0)); + arraylist_free (lines); + + lines = logger_tail_file (filename, 2); + CHECK(lines); + LONGS_EQUAL(2, arraylist_size (lines)); + STRCMP_EQUAL("this is a test, line 999", (const char *)arraylist_get (lines, 0)); + STRCMP_EQUAL("this is a test, line 1000", (const char *)arraylist_get (lines, 1)); + arraylist_free (lines); + + lines = logger_tail_file (filename, 3); + CHECK(lines); + LONGS_EQUAL(3, arraylist_size (lines)); + STRCMP_EQUAL("this is a test, line 998", (const char *)arraylist_get (lines, 0)); + STRCMP_EQUAL("this is a test, line 999", (const char *)arraylist_get (lines, 1)); + STRCMP_EQUAL("this is a test, line 1000", (const char *)arraylist_get (lines, 2)); + arraylist_free (lines); + + lines = logger_tail_file (filename, 4); + CHECK(lines); + LONGS_EQUAL(4, arraylist_size (lines)); + STRCMP_EQUAL("this is a test, line 997", (const char *)arraylist_get (lines, 0)); + STRCMP_EQUAL("this is a test, line 998", (const char *)arraylist_get (lines, 1)); + STRCMP_EQUAL("this is a test, line 999", (const char *)arraylist_get (lines, 2)); + STRCMP_EQUAL("this is a test, line 1000", (const char *)arraylist_get (lines, 3)); + arraylist_free (lines); + + lines = logger_tail_file (filename, 1000); + CHECK(lines); + LONGS_EQUAL(1000, arraylist_size (lines)); + STRCMP_EQUAL("this is a test, line 1", (const char *)arraylist_get (lines, 0)); + STRCMP_EQUAL("this is a test, line 2", (const char *)arraylist_get (lines, 1)); + STRCMP_EQUAL("this is a test, line 3", (const char *)arraylist_get (lines, 2)); + STRCMP_EQUAL("this is a test, line 4", (const char *)arraylist_get (lines, 3)); + STRCMP_EQUAL("this is a test, line 998", (const char *)arraylist_get (lines, 997)); + STRCMP_EQUAL("this is a test, line 999", (const char *)arraylist_get (lines, 998)); + STRCMP_EQUAL("this is a test, line 1000", (const char *)arraylist_get (lines, 999)); + arraylist_free (lines); + + lines = logger_tail_file (filename, 2000); + CHECK(lines); + LONGS_EQUAL(1000, arraylist_size (lines)); + STRCMP_EQUAL("this is a test, line 1", (const char *)arraylist_get (lines, 0)); + STRCMP_EQUAL("this is a test, line 2", (const char *)arraylist_get (lines, 1)); + STRCMP_EQUAL("this is a test, line 3", (const char *)arraylist_get (lines, 2)); + STRCMP_EQUAL("this is a test, line 4", (const char *)arraylist_get (lines, 3)); + STRCMP_EQUAL("this is a test, line 998", (const char *)arraylist_get (lines, 997)); + STRCMP_EQUAL("this is a test, line 999", (const char *)arraylist_get (lines, 998)); + STRCMP_EQUAL("this is a test, line 1000", (const char *)arraylist_get (lines, 999)); + arraylist_free (lines); + + unlink (filename); + free (filename); + + /* write a bigger test file, with empty lines */ + filename = string_eval_path_home ("${weechat_data_dir}/test_file.txt", + NULL, NULL, NULL); + file = fopen (filename, "w"); + for (i = 0; i < 1000; i++) + { + snprintf (line, sizeof (line), "this is a test, line %d\n\n", i + 1); + fwrite (line, 1, strlen (line), file); + } + fflush (file); + fclose (file); + + POINTERS_EQUAL(NULL, logger_tail_file (filename, 0)); + + lines = logger_tail_file (filename, 1); + CHECK(lines); + LONGS_EQUAL(1, arraylist_size (lines)); + STRCMP_EQUAL("", (const char *)arraylist_get (lines, 0)); + arraylist_free (lines); + + lines = logger_tail_file (filename, 2); + CHECK(lines); + LONGS_EQUAL(2, arraylist_size (lines)); + STRCMP_EQUAL("this is a test, line 1000", (const char *)arraylist_get (lines, 0)); + STRCMP_EQUAL("", (const char *)arraylist_get (lines, 1)); + arraylist_free (lines); + + lines = logger_tail_file (filename, 3); + CHECK(lines); + LONGS_EQUAL(3, arraylist_size (lines)); + STRCMP_EQUAL("", (const char *)arraylist_get (lines, 0)); + STRCMP_EQUAL("this is a test, line 1000", (const char *)arraylist_get (lines, 1)); + STRCMP_EQUAL("", (const char *)arraylist_get (lines, 2)); + arraylist_free (lines); + + lines = logger_tail_file (filename, 4); + CHECK(lines); + LONGS_EQUAL(4, arraylist_size (lines)); + STRCMP_EQUAL("this is a test, line 999", (const char *)arraylist_get (lines, 0)); + STRCMP_EQUAL("", (const char *)arraylist_get (lines, 1)); + STRCMP_EQUAL("this is a test, line 1000", (const char *)arraylist_get (lines, 2)); + STRCMP_EQUAL("", (const char *)arraylist_get (lines, 3)); + arraylist_free (lines); + + lines = logger_tail_file (filename, 2000); + CHECK(lines); + LONGS_EQUAL(2000, arraylist_size (lines)); + STRCMP_EQUAL("this is a test, line 1", (const char *)arraylist_get (lines, 0)); + STRCMP_EQUAL("", (const char *)arraylist_get (lines, 1)); + STRCMP_EQUAL("this is a test, line 2", (const char *)arraylist_get (lines, 2)); + STRCMP_EQUAL("", (const char *)arraylist_get (lines, 3)); + STRCMP_EQUAL("this is a test, line 3", (const char *)arraylist_get (lines, 4)); + STRCMP_EQUAL("", (const char *)arraylist_get (lines, 5)); + STRCMP_EQUAL("this is a test, line 998", (const char *)arraylist_get (lines, 1994)); + STRCMP_EQUAL("", (const char *)arraylist_get (lines, 1995)); + STRCMP_EQUAL("this is a test, line 999", (const char *)arraylist_get (lines, 1996)); + STRCMP_EQUAL("", (const char *)arraylist_get (lines, 1997)); + STRCMP_EQUAL("this is a test, line 1000", (const char *)arraylist_get (lines, 1998)); + STRCMP_EQUAL("", (const char *)arraylist_get (lines, 1999)); + arraylist_free (lines); + + lines = logger_tail_file (filename, 4000); + CHECK(lines); + LONGS_EQUAL(2000, arraylist_size (lines)); + STRCMP_EQUAL("this is a test, line 1", (const char *)arraylist_get (lines, 0)); + STRCMP_EQUAL("", (const char *)arraylist_get (lines, 1)); + STRCMP_EQUAL("this is a test, line 2", (const char *)arraylist_get (lines, 2)); + STRCMP_EQUAL("", (const char *)arraylist_get (lines, 3)); + STRCMP_EQUAL("this is a test, line 3", (const char *)arraylist_get (lines, 4)); + STRCMP_EQUAL("", (const char *)arraylist_get (lines, 5)); + STRCMP_EQUAL("this is a test, line 998", (const char *)arraylist_get (lines, 1994)); + STRCMP_EQUAL("", (const char *)arraylist_get (lines, 1995)); + STRCMP_EQUAL("this is a test, line 999", (const char *)arraylist_get (lines, 1996)); + STRCMP_EQUAL("", (const char *)arraylist_get (lines, 1997)); + STRCMP_EQUAL("this is a test, line 1000", (const char *)arraylist_get (lines, 1998)); + STRCMP_EQUAL("", (const char *)arraylist_get (lines, 1999)); + arraylist_free (lines); + + unlink (filename); + free (filename); }