There is a small resource leak in cmd/sort.c:/^dofile. Here is the code with some hopefully explanatory annotations (one liner patch is below): <snip> void dofile(Biobuf *b) { Line *l, *ol; int n; if(args.cflag) { if((ol = newline(b)) == nil) return; for(;;) { if((l = newline(b)) == nil) break; ^^^^^^^^^^^^^^^^^^^^^^ Break out of for leakes 'ol' n = kcmp(ol->key, l->key); if(n > 0 || (n == 0 && args.uflag)) { fprint(2, "sort: -c file not in sort\n"); /**/ done("order"); } free(ol->key); free(ol); ol = l; ^^^^^^^^^^^^^^^^^^^^^^ Assign 'l' to 'ol' and again break out of for would leak that } return; ^^^^^^^^^^^^^^^ There should be a 'free(ol);' before this 'return' } </snip> Here is the one line fix as an inline diff: diff -r 4c6206d69abb sys/src/cmd/sort.c --- a/sys/src/cmd/sort.c Mon Dec 28 21:21:22 2020 +0100 +++ b/sys/src/cmd/sort.c Tue Dec 29 15:11:29 2020 +0100 @@ -209,6 +209,7 @@ free(ol); ol = l; } + free(ol); return; }
Quoth Igor Boehm <boehm.igor@gmail.com>:
> diff -r 4c6206d69abb sys/src/cmd/sort.c
> --- a /sys/src/cmd/sort.c Mon Dec 28 21:21:22 2020 +0100
> +++ b/sys/src/cmd/sort.c Tue Dec 29 15:11:29 2020 +0100
> @@ -209,6 +209,7 @@
> free(ol);
> ol = l;
> }
> + free(ol);
> return;
> }
>
>
Am I wrong in thinking that we should also be
freeing ol->key?
>Quoth Igor Boehm <boehm.igor@gmail.com>:
>> diff -r 4c6206d69abb sys/src/cmd/sort.c
>> --- a /sys/src/cmd/sort.c Mon Dec 28 21:21:22 2020 +0100
>> +++ b/sys/src/cmd/sort.c Tue Dec 29 15:11:29 2020 +0100
>> @@ -209,6 +209,7 @@
>> free(ol);
>> ol = l;
>> }
>> + free(ol);
>> return;
>> }
>
>Am I wrong in thinking that we should also be
>freeing ol->key?
Good catch Ori! Me must have been blind when reviewing this ~SIGH~
Here the reasoning spelled out:
'ol->key' is either heap allocated or set to '0' in function
/sys/src/cmd/sort.c:^buildkey.
The buildkey function is called from /sys/src/cmd/sort.c:^newline and
I believe that on all its exit paths where allocation of the line
succeeds, buildkey() is called.
Hence it is prudent to call free on it as you suggest (if ol->key is
'0' the free() acts as a noop).
So the patch should look like this:
diff -r 4c6206d69abb sys/src/cmd/sort.c
--- a/sys/src/cmd/sort.c Mon Dec 28 21:21:22 2020 +0100
+++ b/sys/src/cmd/sort.c Fri Jan 01 20:28:23 2021 +0100
@@ -209,6 +209,8 @@
free(ol);
ol = l;
}
+ free(ol->key);
+ free(ol);
return;
}
Quoth Igor Boehm <boehm.igor@gmail.com>:
> So the patch should look like this:
>
> diff -r 4c6206d69abb sys/src/cmd/sort.c
> --- a/sys/src/cmd/sort.c Mon Dec 28 21:21:22 2020 +0100
> +++ b/sys/src/cmd/sort.c Fri Jan 01 20:28:23 2021 +0100
> @@ -209,6 +209,8 @@
> free(ol);
> ol = l;
> }
> + free(ol->key);
> + free(ol);
> return;
> }
>
done. first commit of the year :)