Re: [hackers] [PATCH 2/3][sbase] sort: Don't do top-level sort when -k is used

From: Michael Forney <mforney_AT_mforney.org>
Date: Tue, 31 Dec 2019 15:07:30 -0800

On 2019-12-31, Richard Ipsum <richardipsum_AT_vx21.xyz> wrote:
> ---
> sort.c | 14 +++++++++-----
> 1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/sort.c b/sort.c
> index 941b694..e974011 100644
> --- a/sort.c
> +++ b/sort.c
> _AT_@ -196,7 +196,8 @@ slinecmp(struct line *a, struct line *b)
> x = strtod(col1.line.data, NULL);
> y = strtod(col2.line.data, NULL);
> res = (x < y) ? -1 : (x > y);
> - } else if (kd->flags & (MOD_D | MOD_F | MOD_I)) {
> + } else if ((kd->flags & MOD_D) | (kd->flags & MOD_F) |
> + (kd->flags & MOD_I)) {

This is functionally the same, right? Any reason for this change?

> res = skipmodcmp(&col1.line, &col2.line, kd->flags);
> } else {
> res = linecmp(&col1.line, &col2.line);
> _AT_@ -386,10 +387,13 @@ main(int argc, char *argv[])
> usage();
> } ARGEND
>
> - /* -b shall only apply to custom key definitions */
> - if (TAILQ_EMPTY(&kdhead) && global_flags)
> - addkeydef("1", global_flags & ~(MOD_STARTB | MOD_ENDB));
> - addkeydef("1", global_flags & MOD_R);
> + if (TAILQ_EMPTY(&kdhead)) {
> + if (global_flags) {
> + /* -b shall only apply to custom key definitions */
> + addkeydef("1", global_flags & ~(MOD_STARTB | MOD_ENDB));
> + }
> + addkeydef("1", global_flags & MOD_R);
> + }
>
> if (!argc) {
> if (Cflag || cflag) {

We use qsort to sort the lines, and the order of elements that compare
equal when applied to qsort is unspecified, so I think this means that
the output of sbase sort(1) could vary from libc to libc. POSIX says
that `The order in which lines that still compare equal are written is
unspecified`, so I don't think there is actually a conformance issue
with that, but it is something to consider.

I do see the problem with the top-level sort when -[cC] is used, so
maybe another option is to only add it when we are not doing order
checking.

What are your thoughts on this?
Received on Wed Jan 01 2020 - 00:07:30 CET