From 3da509e008f28775c05c2522d113da480123c7ea Mon Sep 17 00:00:00 2001 From: Donald Buczek Date: Wed, 6 Jul 2022 12:36:27 +0200 Subject: [PATCH 1/8] bee_version_compare: Rewrite compare_version_strings() The current algorithm is complicated and broken. First, it removes common prefixes, so that, for example, '1.5.8' and '1.5.9' are compared numerically as 8 and 9. After that, '0's are skipped, to that, for example, '1.5.09' can compare equal to '1.5.9'. In a next step, removed digits are restored, so that that '1.5.1134' and '1.5.1211' are compared as 1134 and 1211 and not as 34 and 11. However, the removal of '0's is done independently for both values, so that a different number of '0's might be removed, while the undo is done in sync, so that the same number of digits is 'restored'. As a result, '1.101' compares less that '1.11': a b 1.101 1.11 # original 01 1 # after removal of common prefix 1 1 # after removal of '0's 01 11 # after restoration of digits Additionally, the code tries to sort digits after non-digit characters but failed to miss the case, when only the second string starts with a digit. Rewrite algorithm from scratch. The new algorithm advances through both strings comparing them at their beginning - if both strings start with a digit - compare numerically - if equal, skip over digits and loop - otherwise if the first character is different, compare character values - otherwise if both strings end, result is 0 (equal) - otherwise, advance to next character and loop We intentionally don't sort digits after non-digits, as the old code tried, because this makes more sense for the case when we have a hex string (e.g. from a git hash) as part of the version string. To make the code more readable, the character pointers are dereferenced multiple times and some conditionals are evaluated multiple times. We can trust the compiler to optimize this away. Comparing 'bee list --available' with the old and the new algorithm produces the following differences: firefox-9.0.1-0.x86_64 firefox-9.0.1-0.x86_64 firefox-10.0.2-0.x86_64 firefox-10.0.2-0.x86_64 firefox-102.0-0.x86_64 < firefox-11.0-0.x86_64 firefox-11.0-0.x86_64 firefox-12.0-0.x86_64 firefox-12.0-0.x86_64 firefox-94.0.1-0.x86_64 firefox-94.0.1-0.x86_64 firefox-94.0.2-0.x86_64 firefox-94.0.2-0.x86_64 > firefox-102.0-0.x86_64 firefox_current-4-0.x86_64 firefox_current-4-0.x86_64 firefox_current-5-0.x86_64 firefox_current-5-0.x86_64 java-1.7.0_13-0.x86_64 java-1.7.0_13-0.x86_64 java-1.7.0_17-0.x86_64 java-1.7.0_17-0.x86_64 > java-1.8.0_11-0.x86_64 > java-1.8.0_45-0.x86_64 java-1.8.0_101-0.x86_64 java-1.8.0_101-0.x86_64 java-1.8.0_102-0.x86_64 java-1.8.0_102-0.x86_64 java-1.8.0_102-1.x86_64 java-1.8.0_102-1.x86_64 java-1.8.0_11-0.x86_64 < java-1.8.0_45-0.x86_64 < java-1.8.0_121-0.x86_64 java-1.8.0_121-0.x86_64 java-1.8.0_131-0.x86_64 java-1.8.0_131-0.x86_64 mxq-0.0_p106_8a0ad87-0.x86_64 mxq-0.0_p106_8a0ad87-0.x86_64 mxq-0.0_p107_eaf8146-0.x86_64 mxq-0.0_p107_eaf8146-0.x86_64 mxq-0.0_p108_f56522b-0.x86_64 < mxq-0.0_p108_0b7afc1-0.x86_64 mxq-0.0_p108_0b7afc1-0.x86_64 mxq-0.0_p108_4e83c8d-0.x86_64 mxq-0.0_p108_4e83c8d-0.x86_64 mxq-0.0_p109_ed52e7f-0.x86_64 | mxq-0.0_p108_f56522b-0.x86_64 mxq-0.0_p109_621fea8-0.x86_64 mxq-0.0_p109_621fea8-0.x86_64 > mxq-0.0_p109_ed52e7f-0.x86_64 mxq-0.0_p110_baba367-0.x86_64 mxq-0.0_p110_baba367-0.x86_64 mxq-0.0_p111_38cc7db-0.x86_64 mxq-0.0_p111_38cc7db-0.x86_64 mxq-0.1.1_p1_4840161-0.x86_64 mxq-0.1.1_p1_4840161-0.x86_64 mxq-0.1.2_p0_d21c6ae-0.x86_64 mxq-0.1.2_p0_d21c6ae-0.x86_64 mxq-0.1.2_p1_efa8ec4-0.x86_64 < mxq-0.1.2_p1_9ccc12f-0.x86_64 mxq-0.1.2_p1_9ccc12f-0.x86_64 > mxq-0.1.2_p1_efa8ec4-0.x86_64 mxq-0.1.3_p0_0c3032c-0.x86_64 mxq-0.1.3_p0_0c3032c-0.x86_64 mxq-0.1.4_p0_c936ba0-0.x86_64 mxq-0.1.4_p0_c936ba0-0.x86_64 mxstartup-2.9_p1_0e23fa8-0.x86_64 mxstartup-2.9_p1_0e23fa8-0.x86_64 mxstartup-2.9_p2_e915bc3-0.x86_64 mxstartup-2.9_p2_e915bc3-0.x86_64 mxstartup-2.9_p3_d919716-0.x86_64 < mxstartup-2.9_p3_21347ab-0.x86_64 mxstartup-2.9_p3_21347ab-0.x86_64 > mxstartup-2.9_p3_d919716-0.x86_64 mxstartup-2.10_p0_d919716-0.x86_64 mxstartup-2.10_p0_d919716-0.x86_64 mxstartup-2.11_p0_4ee8fed-0.x86_64 mxstartup-2.11_p0_4ee8fed-0.x86_64 --- src/bee_version_compare.c | 100 ++++++++++---------------------------- 1 file changed, 27 insertions(+), 73 deletions(-) diff --git a/src/bee_version_compare.c b/src/bee_version_compare.c index 43824b8..9706228 100644 --- a/src/bee_version_compare.c +++ b/src/bee_version_compare.c @@ -28,86 +28,40 @@ #include "bee_version.h" -int compare_version_strings(char *v1, char *v2) { - char *a, *b; - long long i,j; - assert(v1); - assert(v2); - - a = v1; - b = v2; - - while(*a && *b && *a == *b) { - a++; - b++; - - if (*a == *b) +// '2.23.2' > '2.23.0' +//' 2.23.10' > '2.23.8' +// '2.23.001' == '2.23.1' +// '2.23.1001' > '2.23.101' + +int compare_version_strings(char *a, char *b) { + while (1) { + + if (isdigit(*a) && isdigit (*b)) { + long i_a = atoll(a); + long i_b = atoll(b); + if (i_a < i_b) + return -1; + if (i_a > i_b) + return 1; + while(isdigit(*a)) + a++; + while(isdigit(*b)) + b++; continue; - - /* skip leading zeros of numbers != 0 */ - if (isdigit(*a) && isdigit(*b)) { - char *c; - - for (c=a; *c == '0'; c++) - ; - if (isdigit(*c)) - a = c; - - for (c=b; *c == '0'; c++) - ; - if (isdigit(*c)) - b = c; - } - } - - /* strings are equal ; *a==*b==0*/ - if(*a == *b) - return(0); - - if(isdigit(*a)) { - if(isdigit(*b)) { - /* rewind string to first digit */ - /* e.g. to compare 12 vs 100 and not 2 vs 00 */ - while(a > v1 && isdigit(*(a-1)) && - b > v2 && isdigit(*(b-1))) { - a--; - b--; - } - i = atoll(a); - j = atoll(b); - - if(ij) - return(1); - - /* numbers are equal but strings are not? */ - /* yes -> leading zeros: atoll("01") == atoll("1") */ - return(0); } - /* a > ('.',alpha, 0) */ - return(1); - } - if(isalpha(*a)) { + if (*a < *b) + return -1; + if (*a > *b) + return 1; - /* alpha < digit */ - if(isdigit(*b)) - return(-1); + if (*a == '\0') // implies *b == '\0', too + return 0; - if(isalpha(*b)) { - if(*a < *b) - return(-1); - return(1); - } - return(1); + a++; + b++; } - - if(! *b) - return(1); - - return(-1); } int compare_beepackage_names(struct beeversion *v1, struct beeversion *v2) { From c9df684507b4e000a9c1fdb1fef422a126fa4d63 Mon Sep 17 00:00:00 2001 From: Donald Buczek Date: Fri, 8 Jul 2022 07:24:40 +0200 Subject: [PATCH 2/8] bee_version_compare: Make compare_version_strings static --- src/bee_version_compare.c | 2 +- src/bee_version_compare.h | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/bee_version_compare.c b/src/bee_version_compare.c index 9706228..6514fe8 100644 --- a/src/bee_version_compare.c +++ b/src/bee_version_compare.c @@ -34,7 +34,7 @@ // '2.23.001' == '2.23.1' // '2.23.1001' > '2.23.101' -int compare_version_strings(char *a, char *b) { +static int compare_version_strings(char *a, char *b) { while (1) { if (isdigit(*a) && isdigit (*b)) { diff --git a/src/bee_version_compare.h b/src/bee_version_compare.h index 5af38ef..3a333fb 100644 --- a/src/bee_version_compare.h +++ b/src/bee_version_compare.h @@ -24,7 +24,6 @@ #include "bee_version.h" -int compare_version_strings(char *v1, char *v2); int compare_beepackage_names(struct beeversion *v1, struct beeversion *v2); int compare_beeversions(struct beeversion *v1, struct beeversion *v2); int compare_beepackages(struct beeversion *v1, struct beeversion *v2); From e2fc142a3b7c588d5756744c9968796d76f41e3c Mon Sep 17 00:00:00 2001 From: Donald Buczek Date: Fri, 8 Jul 2022 07:26:25 +0200 Subject: [PATCH 3/8] Makefile: Add -O3 --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 514b753..1312b9f 100644 --- a/Makefile +++ b/Makefile @@ -1,7 +1,7 @@ BEE_VERSION = 1.2.24 CC=gcc -CFLAGS=-Wall -g +CFLAGS=-Wall -g -O3 LDFLAGS= PREFIX = /usr From 30e09ca6453f5d62f00ecfb3c593d44e0ef51877 Mon Sep 17 00:00:00 2001 From: Donald Buczek Date: Fri, 8 Jul 2022 07:49:48 +0200 Subject: [PATCH 4/8] beeflock: Remove unused flag argument --- src/beeflock.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/beeflock.c b/src/beeflock.c index f663a0f..95bcb1a 100644 --- a/src/beeflock.c +++ b/src/beeflock.c @@ -66,7 +66,7 @@ void usage(void) #define BEEFLOCK_UNLOCK LOCK_UN #define BEEFLOCK_NOBLOCK LOCK_NB -int bee_flock_fd(int fd, int operation, int flags) +int bee_flock_fd(int fd, int operation) { int res; @@ -123,7 +123,7 @@ int bee_flock_close(int fd) return res; } -int bee_flock(char *filename, int operation, int flags) +int bee_flock(char *filename, int operation) { int res; int fd, fd2; @@ -134,7 +134,7 @@ int bee_flock(char *filename, int operation, int flags) return -1; while (1) { - res = bee_flock_fd(fd, operation, 0); + res = bee_flock_fd(fd, operation); if (res < 0) return -1; @@ -169,7 +169,7 @@ int bee_flock(char *filename, int operation, int flags) } -int bee_execute_command(char *command[], int flags) +int bee_execute_command(char *command[]) { pid_t fpid, wpid; int wstatus; @@ -265,12 +265,12 @@ int main(int argc, char *argv[]) BEE_EXIT(OSERR); } - fd = bee_flock(lockfilename, operation, 0); + fd = bee_flock(lockfilename, operation); if (fd < 0) { BEE_EXIT(SOFTWARE); } - res = bee_execute_command(command, 0); + res = bee_execute_command(command); bee_flock_close(fd); if (res < 0) { BEE_EXIT(SOFTWARE); From d1524e3afaf619e105633d04a9ad6e70ea2dccca Mon Sep 17 00:00:00 2001 From: Donald Buczek Date: Fri, 8 Jul 2022 07:52:44 +0200 Subject: [PATCH 5/8] bee_sort: Annotate unused parameter --- src/beesort.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/beesort.c b/src/beesort.c index b8fa6b4..69fe110 100644 --- a/src/beesort.c +++ b/src/beesort.c @@ -61,6 +61,7 @@ int my_compare_data(void *a, void *b) void my_print(void *key, void *data) { + (void)key; fputs(data, stdout); } From 50262317d9cd898cd99ef1f6d79c4f47280aeae3 Mon Sep 17 00:00:00 2001 From: Donald Buczek Date: Fri, 8 Jul 2022 08:01:39 +0200 Subject: [PATCH 6/8] tree: Add fall through annotations Gcc can warn on implicit fallthroughs. Mark them with a comment which is recognized by gcc. --- src/bee_version_output.c | 1 + src/beegetopt.c | 1 + 2 files changed, 2 insertions(+) diff --git a/src/bee_version_output.c b/src/bee_version_output.c index aa67df5..26443ac 100644 --- a/src/bee_version_output.c +++ b/src/bee_version_output.c @@ -156,6 +156,7 @@ void print_format(char* s, struct beeversion *v, char *filter_pkgfullname) p++; continue; } + // fall through default: printf("%%%c", *p); break; diff --git a/src/beegetopt.c b/src/beegetopt.c index c82acf1..8889b81 100644 --- a/src/beegetopt.c +++ b/src/beegetopt.c @@ -114,6 +114,7 @@ int main(int argc, char *argv[]) switch(opt) { case 'V': printf("beegetopt Vx.x\n"); + // fall through case 'h': usage(); exit(0); From d09d0cddd47d4fa2ecca610c3c7e8685fedc909f Mon Sep 17 00:00:00 2001 From: Donald Buczek Date: Fri, 8 Jul 2022 08:04:38 +0200 Subject: [PATCH 7/8] beesep: Don't compare expressions of different signedness --- src/beesep.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/beesep.c b/src/beesep.c index e7aaa60..1e13135 100644 --- a/src/beesep.c +++ b/src/beesep.c @@ -70,7 +70,7 @@ static void print_escaped(char *s, size_t n) bee_fprint(stdout, "'"); - while ((c = strchr(s, '\'')) && c - s < n) { + while ((c = strchr(s, '\'')) && c < s + n) { if (c-s) bee_fnprint(stdout, c - s, s); bee_fprint(stdout, "'\\''"); From 105ddb49ee80d0a785b1c3bd66723e0139aa883e Mon Sep 17 00:00:00 2001 From: Donald Buczek Date: Fri, 8 Jul 2022 08:06:12 +0200 Subject: [PATCH 8/8] Makefile: Add -Wextra -Wno-override-init -Werror Enable -Wextra. Exclude -Wno-override-init for now, because bee_getopt.h relies on it. See [1] for similar issue in mxq. [1]: https://github.molgen.mpg.de/mariux64/mxq/issues/131 --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 1312b9f..096caa1 100644 --- a/Makefile +++ b/Makefile @@ -1,7 +1,7 @@ BEE_VERSION = 1.2.24 CC=gcc -CFLAGS=-Wall -g -O3 +CFLAGS=-Wall -Wextra -Wno-override-init -Werror -g -O3 LDFLAGS= PREFIX = /usr