9front - general discussion about 9front
 help / color / mirror / Atom feed
* [9front] cmd/sort: fix resource leak (patch)
@ 2020-12-29 14:28 Igor Boehm
  2021-01-01 17:42 ` ori
  0 siblings, 1 reply; 4+ messages in thread
From: Igor Boehm @ 2020-12-29 14:28 UTC (permalink / raw)
  To: 9front; +Cc: boehm.igor

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


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [9front] cmd/sort: fix resource leak (patch)
  2020-12-29 14:28 [9front] cmd/sort: fix resource leak (patch) Igor Boehm
@ 2021-01-01 17:42 ` ori
  2021-01-01 19:32   ` Igor Boehm
  0 siblings, 1 reply; 4+ messages in thread
From: ori @ 2021-01-01 17:42 UTC (permalink / raw)
  To: 9front; +Cc: 9front

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?

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [9front] cmd/sort: fix resource leak (patch)
  2021-01-01 17:42 ` ori
@ 2021-01-01 19:32   ` Igor Boehm
  2021-01-01 19:49     ` ori
  0 siblings, 1 reply; 4+ messages in thread
From: Igor Boehm @ 2021-01-01 19:32 UTC (permalink / raw)
  To: 9front

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


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [9front] cmd/sort: fix resource leak (patch)
  2021-01-01 19:32   ` Igor Boehm
@ 2021-01-01 19:49     ` ori
  0 siblings, 0 replies; 4+ messages in thread
From: ori @ 2021-01-01 19:49 UTC (permalink / raw)
  To: 9front

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-01-01 19:52 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-29 14:28 [9front] cmd/sort: fix resource leak (patch) Igor Boehm
2021-01-01 17:42 ` ori
2021-01-01 19:32   ` Igor Boehm
2021-01-01 19:49     ` ori

9front - general discussion about 9front

This inbox may be cloned and mirrored by anyone:

	git clone --mirror http://inbox.vuxu.org/9front

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V1 9front 9front/ http://inbox.vuxu.org/9front \
		9front@9front.org
	public-inbox-index 9front

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.vuxu.org/vuxu.archive.9front


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git