Re: [PATCH] Fix --reuse-cmdline so it is usable.

[ebiederm@xmission.com (Eric W. Biederman)]

Simon Horman <horms@verge.net.au> writes:

> On Tue, Jan 19, 2010 at 12:05:03AM -0800, Eric W. Biederman wrote:
>> 
>> A colleague of mine implemented kdump and it used --reuse-cmdline
>> with some rather interesting and unexpected results.
>> 
>> Update the getopt specification so that --reuse-cmdline does not
>> attempt to take an argument that it will not use.
>> 
>> Update the processing of --append so that --reuse-cmdline followed
>> by --append actually appends the parameters specified by --reuse-cmdline.
>
> Hi Eric,
>
> sorry for being slow. Been semi-offline for LCA and am now catching
> up on things.

No problem, I am pretty out of it right now as well.

> [snip]
>
>> diff --git a/kexec/kexec.c b/kexec/kexec.c
>> index a1cec86..f4c22a6 100644
>> --- a/kexec/kexec.c
>> +++ b/kexec/kexec.c
>> @@ -994,6 +994,22 @@ void check_reuse_initrd(void)
>>  	free(line);
>>  }
>>  
>> +const char *concat_cmdline(const char *base, const char *append)
>> +{
>> +	const char *cmdline;
>> +	if (!base && !append)
>> +		return NULL;
>> +	if (!base)
>> +		return append;
>> +	if (!append)
>> +		return base;
>> +	cmdline = xmalloc(strlen(base) + 1 + strlen(append) + 1);
>> +	strcpy(cmdline, base);
>> +	strcat(cmdline, " ");
>> +	strcat(cmdline, append);
>> +	return cmdline;
>> +}
>> +
>
> This introduces a memory leak.

Yep.

> Perhaps it should strdup append and base in the !base and !append cases
> respectively and expect the caller to always call free.
>
> I realise that its a small leak in a programme that will soon exit anyway.
> But for the sake of being able to use tools like valgrind to analyse
> problems it seems to me that leaks are worth avoiding.  (Not that I have
> run valgrind on kexec-tools to see what happens :-)

I see your point but I think we already have a memory leak here (
Where does the memory that getopt uses come from? ), and I think on a
trivial application like /sbin/kexec that is simply not long running
it can't matter.  I'm even willing to call not freeing memory
explicitly a performance optimization in cases like this ;)

Eric


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec