Re: [Fastboot] [PATCH] Kexec: Common alloc

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

Vivek Goyal <vgoyal@in.ibm.com> writes:

> On Wed, Apr 12, 2006 at 05:33:02PM +0900, Magnus Damm wrote:
>> Kexec: Common alloc
>> 
>> This patch reduces code redundancy by introducing a new function called
>> kimage_common_alloc() which is used to set up image->control_code_page.
>> 
>> Signed-off-by: Magnus Damm <magnus@valinux.co.jp>
>> ---
>> 
>> Applies on top of linux-2.6.17-rc1-git5 + "Kexec: Remove duplicate rimage"
>> 
>>  kexec.c |   51 ++++++++++++++++++++-------------------------------
>>  1 files changed, 20 insertions(+), 31 deletions(-)
>> 
>> --- 0004/kernel/kexec.c
>> +++ work/kernel/kexec.c	2006-04-12 16:30:34.000000000 +0900
>> @@ -205,34 +205,36 @@ out:
>>  
>>  }
>>  
>> -static int kimage_normal_alloc(struct kimage **rimage, unsigned long entry,
>> -				unsigned long nr_segments,
>> -				struct kexec_segment __user *segments)
>> +static int kimage_common_alloc(struct kimage *image)
>>  {
>> -	int result;
>> -	struct kimage *image;
>> -
>> -	/* Allocate and initialize a controlling structure */
>> -	image = NULL;
>> -	result = do_kimage_alloc(&image, entry, nr_segments, segments);
>> -	if (result)
>> -		goto out;
>> -
>>  	/*
>> -	 * Find a location for the control code buffer, and add it
>> +	 * Find a location for the control code buffer, and add
>>  	 * the vector of segments so that it's pages will also be
>>  	 * counted as destination pages.
>>  	 */
>> -	result = -ENOMEM;
>>  	image->control_code_page = kimage_alloc_control_pages(image,
>>  					   get_order(KEXEC_CONTROL_CODE_SIZE));
>>  	if (!image->control_code_page) {
>>  		printk(KERN_ERR "Could not allocate control_code_buffer\n");
>> -		goto out;
>> +		return -ENOMEM;
>
> Ok. So effectively call to the the function kimage_alloc_control_pages() and
> its return code handling is being wrapped in another function. This function
> is called at only two places. Not quite convinced that this duplication is
> significant enough that we introduce another function to wrap a function
> call.

Agreed.

Eric