The memory leak was an off-by-one-error, I can't reproduce it after
removing some needless pointer arithmetic.
Yes, I had some constructive pointers about semantic diffing between the
two previous patches.
The attached patch looks like this:
[image: 2022-08-18T155817.png]
Your examples would go
5.18.*14.arch1-1*       5.18.*15arch1-1*
5.18.*9*                      5.18.*10*

To me, this is very useful. Especially when there's many rows, it helps
when figuring out which release notes to read after an upgrade.
Disabling `color` or `verbose` of course removes this.



Den sön 31 juli 2022 kl 07:05 skrev Allan McRae <[email protected]>:

> On 1/3/22 15:02, David Gustavsson wrote:
> > Use non-alphanumerics as branching points for diff coloring.
> >
> > Signed-off-by: David Gustavsson <[email protected]>
> > ---
>
> There is a memory leak somewhere in here:
>
> $ sudo ./build/pacman -Su
> :: Starting full system upgrade...
> resolving dependencies...
> looking for conflicting packages...
> free(): invalid next size (fast)
> Aborted
>
>
> I have not looked for it yet, because I am still deciding the utility of
> these patches.
>
>
> For discussion, consider these versions:
>
> 5.18.14.arch1-1
> 5.18.15.arch1-1
>
> With the first patch in this series applied (have not looked to see if
> this changes with the second), this starts highlighting after "5.18.1".
>   But if we were going from 5.18.9 to 5.18.10, it would highlight after
> the "5.18.".  These upgrades are no different in that versioning scheme.
>   This should definitely always highlight from the version level separator.
>
>
> Also, I personally do not find this colouring useful, but I also do not
> find the verbose package lists before update useful...  So I am open to
> opinions on this.  Here is an example of the output after the patch 1/2
> applied:
> http://allanmcrae.com/tmp/pacman-version-change-highlight.png
>
> I'll wait for more feedback before looking into this further.
>
> Allan
>
>
>
>
>
From 823add4685b99c18cb2bb46b50c6d2403fcf31ff Mon Sep 17 00:00:00 2001
From: David Gustavsson <[email protected]>
Date: Wed, 17 Aug 2022 14:54:03 +0200
Subject: [PATCH] Version diff coloring

---
 src/pacman/conf.c |  4 +++
 src/pacman/conf.h |  2 ++
 src/pacman/util.c | 80 ++++++++++++++++++++++++++++++++++++++++++-----
 src/pacman/util.h |  2 ++
 4 files changed, 81 insertions(+), 7 deletions(-)

diff --git a/src/pacman/conf.c b/src/pacman/conf.c
index f9edf75b..d5c598b9 100644
--- a/src/pacman/conf.c
+++ b/src/pacman/conf.c
@@ -80,6 +80,8 @@ void enable_colors(int colors)
 		colstr->err     = BOLDRED;
 		colstr->faint   = GREY46;
 		colstr->nocolor = NOCOLOR;
+		colstr->diffrem = RED;
+		colstr->diffadd = GREEN;
 	} else {
 		colstr->colon   = ":: ";
 		colstr->title   = "";
@@ -91,6 +93,8 @@ void enable_colors(int colors)
 		colstr->err     = "";
 		colstr->faint   = "";
 		colstr->nocolor = "";
+		colstr->diffrem = "";
+		colstr->diffadd = "";
 	}
 }
 
diff --git a/src/pacman/conf.h b/src/pacman/conf.h
index f7916ca9..782b5758 100644
--- a/src/pacman/conf.h
+++ b/src/pacman/conf.h
@@ -33,6 +33,8 @@ typedef struct __colstr_t {
 	const char *err;
 	const char *faint;
 	const char *nocolor;
+	const char *diffrem;
+	const char *diffadd;
 } colstr_t;
 
 typedef struct __config_repo_t {
diff --git a/src/pacman/util.c b/src/pacman/util.c
index 5f5c7c54..5f2c7c3f 100644
--- a/src/pacman/util.c
+++ b/src/pacman/util.c
@@ -34,6 +34,7 @@
 #include <limits.h>
 #include <wchar.h>
 #include <wctype.h>
+#include <ctype.h>
 #ifdef HAVE_TERMIOS_H
 #include <termios.h> /* tcflush */
 #endif
@@ -285,8 +286,7 @@ int rmrf(const char *path)
 	}
 }
 
-/* output a string, but wrap words properly with a specified indentation
- */
+/* output a string, but wrap words properly with a specified indentation */
 void indentprint(const char *str, unsigned short indent, unsigned short cols)
 {
 	wchar_t *wcstr;
@@ -869,15 +869,21 @@ static alpm_list_t *create_verbose_row(pm_target_t *target)
 {
 	char *str;
 	off_t size = 0;
+	size_t split;
 	double human_size;
 	const char *label;
+	const char *old_version_in;
+	const char *new_version_in;
+	char *old_version;
+	char *new_version;
 	alpm_list_t *ret = NULL;
 
 	/* a row consists of the package name, */
 	if(target->install) {
 		const alpm_db_t *db = alpm_pkg_get_db(target->install);
 		if(db) {
-			pm_asprintf(&str, "%s/%s", alpm_db_get_name(db), alpm_pkg_get_name(target->install));
+			pm_asprintf(&str, "%s/%s", alpm_db_get_name(db),
+					alpm_pkg_get_name(target->install));
 		} else {
 			pm_asprintf(&str, "%s", alpm_pkg_get_name(target->install));
 		}
@@ -887,14 +893,43 @@ static alpm_list_t *create_verbose_row(pm_target_t *target)
 	add_table_cell(&ret, str, CELL_NORMAL | CELL_FREE);
 
 	/* old and new versions */
-	pm_asprintf(&str, "%s",
-			target->remove != NULL ? alpm_pkg_get_version(target->remove) : "");
+	if(target->remove) {
+		old_version_in = alpm_pkg_get_version(target->remove);
+	} else {
+		old_version_in = "";
+	}
+	old_version = malloc((strlen(old_version_in) + strlen(config->colstr.diffrem)
+				+ strlen(config->colstr.nocolor) + 1)*sizeof(char));
+	strcpy(old_version, (char *)old_version_in);
+
+	if(target->install) {
+		new_version_in = alpm_pkg_get_version(target->install);
+	} else {
+		new_version_in = "";
+	}
+	new_version = malloc((strlen(new_version_in) + strlen(config->colstr.diffadd)
+				+ strlen(config->colstr.nocolor) + 1)*sizeof(char));
+	strcpy(new_version, (char *)new_version_in);
+
+	split = alpm_diff(old_version_in, new_version_in);
+	if(strlen(old_version_in) > 0) {
+		insertstring(old_version, config->colstr.diffrem, split);
+		strcpy(old_version + strlen(old_version), config->colstr.nocolor);
+	}
+	if(strlen(new_version_in) > 0) {
+		insertstring(new_version, config->colstr.diffadd, split);
+		strcpy(new_version + strlen(new_version), config->colstr.nocolor);
+	}
+
+	pm_asprintf(&str, "%s", old_version);
 	add_table_cell(&ret, str, CELL_NORMAL | CELL_FREE);
 
-	pm_asprintf(&str, "%s",
-			target->install != NULL ? alpm_pkg_get_version(target->install) : "");
+	pm_asprintf(&str, "%s", new_version);
 	add_table_cell(&ret, str, CELL_NORMAL | CELL_FREE);
 
+	free(old_version);
+	free(new_version);
+
 	/* and size */
 	size -= target->remove ? alpm_pkg_get_isize(target->remove) : 0;
 	size += target->install ? alpm_pkg_get_isize(target->install) : 0;
@@ -1942,3 +1977,34 @@ void console_erase_line(void)
 {
 	printf("\x1B[K");
 }
+
+/* inserts string insert into string before index i, modifying string */
+void insertstring(char *string, const char *insert, size_t i)
+{
+	char *temp = malloc((strlen(string + i) + 1)*sizeof(char));
+	strcpy(temp, string + i);
+	strcpy(string + i, insert);
+	strcpy(string + i + strlen(insert), temp);
+	free(temp);
+}
+
+size_t alpm_diff(const char *a, const char *b)
+{
+	/*
+	 * Find the index where `a` and `b` differ
+	 */
+	size_t i = 0;
+	size_t split = 0;
+	while(a[i] == b[i]) {
+		if(a[i] == '\0') {
+			split = i+1;
+			return split;
+		}
+		if(isalnum(a[i]) == 0) {
+			/* Non-alphanumeric character */
+			split = i+1;
+		}
+		i++;
+	}
+	return split;
+}
diff --git a/src/pacman/util.h b/src/pacman/util.h
index 52e79915..cd474996 100644
--- a/src/pacman/util.h
+++ b/src/pacman/util.h
@@ -88,6 +88,8 @@ void console_cursor_move_down(unsigned int lines);
 void console_cursor_move_end(void);
 /* Erases line from the current cursor position till the end of the line */
 void console_erase_line(void);
+void insertstring(char *string, const char *insert, size_t i);
+size_t alpm_diff(const char *a, const char *b);
 
 int pm_printf(alpm_loglevel_t level, const char *format, ...) __attribute__((format(printf,2,3)));
 int pm_asprintf(char **string, const char *format, ...) __attribute__((format(printf,2,3)));
-- 
2.37.2

Reply via email to