Re: [hackers] [PATCH 1/3][sbase] sort: Fix handling of -k n,n case

From: Michael Forney <mforney_AT_mforney.org>
Date: Tue, 31 Dec 2019 14:35:57 -0800

On 2019-12-31, Richard Ipsum <richardipsum_AT_vx21.xyz> wrote:
> When field_start is equal to field_end use field N as the sort key.

I think this is not just a problem with `-k n,n`, but any time
field_end is specified. Only up to the beginning of the end field is
compared (if there are more fields after it).

For example

$ sort -t , -k 1,2 -k 4 <<EOF
a,1,foo,x
a,0,foo,y
EOF
a,1,foo,x
a,0,foo,y
$

This is hidden somewhat because of the implicit top-level keydef added
(fixed in your patch 2).

> diff --git a/sort.c b/sort.c
> index 9706e8f..941b694 100644
> --- a/sort.c
> +++ b/sort.c
> _AT_@ -70,6 +70,9 @@ skipcolumn(struct line *a, int skip_to_next_col)
> s += fieldseplen;
> a->data = s;
> a->len = a->len - (s - a->data);
> + } else {
> + a->data = s;
> + a->len = 1;
> }
> } else {
> a->data += a->len - 1;

I think we should keep the invariant that a->data + a->len is the end
of the line, even though end.len isn't used after the call to
skipcolumn(&end, 0).

What do you think about the following diff instead?

diff --git a/sort.c b/sort.c
index dfc383f..a51997f 100644
--- a/sort.c
+++ b/sort.c
_AT_@ -66,11 +66,10 @@ skipcolumn(struct line *a, int skip_to_next_col)

         if (fieldsep) {
                 if ((s = memmem(a->data, a->len, fieldsep, fieldseplen))) {
- if (skip_to_next_col) {
+ if (skip_to_next_col)
                                 s += fieldseplen;
- a->len -= s - a->data;
- a->data = s;
- }
+ a->len -= s - a->data;
+ a->data = s;
                 } else {
                         a->data += a->len - 1;
                         a->len = 1;
Received on Tue Dec 31 2019 - 23:35:57 CET