Simon Horman <horms at 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
