[9front] cmd/sort: fix resource leak (patch)

[9front] cmd/sort: fix resource leak (patch)

[Igor Boehm]

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;
 	}
 

Re: [9front] cmd/sort: fix resource leak (patch)

[ori]

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?

Re: [9front] cmd/sort: fix resource leak (patch)

[Igor Boehm]

>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;
 	}

Re: [9front] cmd/sort: fix resource leak (patch)

[ori]

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 :)