[PATCH] sort: compute -k END.ENDCHAR from the end field, not the line start

Ali Ahmet Memis via busybox <[email protected]>
Newsgroups gmane.linux.busybox
Message-ID <[email protected]>
get_key() locates a key in two passes: j=0 walks to the start of the
START field, j=1 walks to the end of the END field. When END carries
an ENDCHAR (-kSTART,N.ENDCHAR), that char position is supposed to be
counted from the start of field N. Instead the code overwrote 'end'
with the raw parsed value, an offset from the very start of the line:

	if (key->range[3]) {
		end = key->range[3];

For END field 1 this happens to be the same thing, but for any later
field it is not. The result usually lands before 'start', which then
gets clamped to 'start', producing an empty key. Sort has an ASCII
whole-line fallback when keys tie, so it silently sorts by the whole
line instead of the requested key:

	$ printf '%s\n' 'b:aa' 'a:ba' 'a:aa' | busybox sort -t : -k 2.1,2.1
	a:aa
	a:ba
	b:aa
	$ printf '%s\n' 'b:aa' 'a:ba' 'a:aa' | sort -t : -k 2.1,2.1
	a:aa
	b:aa
	a:ba

Make the j=1 pass stop at the start of field N instead of its end when
ENDCHAR is given, one field-skip short of the usual pass, and add the
ENDCHAR offset from there. This mirrors how STARTCHAR is already
applied relative to the start of the START field. Verified against
several -k combinations (endchar past the field's own length, multiple
fields skipped, startchar and endchar together) matching GNU sort.

Add a regression test; the existing ENDCHAR test only exercises field
1, where the old absolute offset and the correct field-relative one
happen to coincide.

   text    data     bss     dec     hex filename
  12208     848     128   13184    3380 coreutils/sort.o before
  12369     848     128   13345    3421 coreutils/sort.o after
                           +161

Signed-off-by: Ali Ahmet Memis <[email protected]>
---
 coreutils/sort.c     | 15 +++++++++++----
 testsuite/sort.tests | 10 ++++++++++
 2 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/coreutils/sort.c b/coreutils/sort.c
index 2e952f81c..54852d76f 100644
--- a/coreutils/sort.c
+++ b/coreutils/sort.c
@@ -163,9 +163,14 @@ static char *get_key(char *str, struct sort_key *key, int flags)
 		/* Loop through fields */
 		else {
 			unsigned char ch = 0;
+			/* -kSTART,N.ENDCHAR: stop at the start of field N instead
+			 * of its end, ENDCHAR below is an offset from there */
+			unsigned fields = key->range[2*j] + j;
+			if (j && key->range[3])
+				fields--;
 
 			end = 0;
-			for (i = 1; i < key->range[2*j] + j; i++) {
+			for (i = 1; i < fields; i++) {
 				if (key_separator) {
 					/* Skip body of key and separator */
 					while ((ch = str[end]) != '\0') {
@@ -186,7 +191,7 @@ static char *get_key(char *str, struct sort_key *key, int flags)
 				}
 			}
 			/* Remove last delim: "abc:def:" => "abc:def" */
-			if (j && ch) {
+			if (j && ch && !key->range[3]) {
 				//if (str[end-1] != key_separator)
 				//  bb_error_msg(_and_die("BUG! "
 				//  "str[start:%d,end:%d]:'%.*s'",
@@ -203,9 +208,11 @@ static char *get_key(char *str, struct sort_key *key, int flags)
 	/* Strip trailing whitespace if necessary */
 	if (flags & FLAG_bb)
 		while (end > start && isspace(str[end-1])) end--;
-	/* -kSTART,N.ENDCHAR: honor ENDCHAR (1-based) */
+	/* -kSTART,N.ENDCHAR: honor ENDCHAR (1-based), counted from the
+	 * start of field N, which the loop above stopped at
+	 */
 	if (key->range[3]) {
-		end = key->range[3];
+		end += key->range[3];
 		if (end > len) end = len;
 	}
 	/* -kN.STARTCHAR[,...]: honor STARTCHAR (1-based) */
diff --git a/testsuite/sort.tests b/testsuite/sort.tests
index 8dbadbdae..6d12eb6f4 100755
--- a/testsuite/sort.tests
+++ b/testsuite/sort.tests
@@ -114,6 +114,16 @@ aa.2
 ab.1
 " ""
 
+testing "sort with ENDCHAR on a later field" "sort -t: -k2.1,2.1 input" "\
+a:aa
+b:aa
+a:ba
+" "\
+b:aa
+a:ba
+a:aa
+" ""
+
 testing "glibc build sort" "sort -t. -k 1,1 -k 2n,2n -k 3 input" "\
 GLIBC_2.1
 GLIBC_2.1.1
-- 
2.55.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.