Syslinux - Sep 2013 - [PATCH 1/2] com32/lib/: Avoid unneeded allocation.

eparam will only be used if EBIOS is available on the underlying disk.
If not so, then there is no reason to allocate eparam.
---
 com32/lib/syslinux/disk.c |   13 +++++++------
 1 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/com32/lib/syslinux/disk.c b/com32/lib/syslinux/disk.c
index 093751a..554bed3 100644
--- a/com32/lib/syslinux/disk.c
+++ b/com32/lib/syslinux/disk.c
@@ -73,7 +73,7 @@ int disk_int13_retry(const com32sys_t * inreg, com32sys_t *
outreg)
 int disk_get_params(int disk, struct disk_info *const diskinfo)
 {
     static com32sys_t inreg, outreg;
-    struct disk_ebios_eparam *eparam;
+    struct disk_ebios_eparam *eparam = NULL;
     int rv = 0;
 
     memset(diskinfo, 0, sizeof *diskinfo);
@@ -94,12 +94,12 @@ int disk_get_params(int disk, struct disk_info *const
diskinfo)
 	diskinfo->ebios = 1;
     }
 
-    eparam = lmalloc(sizeof *eparam);
-    if (!eparam)
-	return -1;
-
     /* Get extended disk parameters if ebios == 1 */
     if (diskinfo->ebios) {
+	eparam = lmalloc(sizeof *eparam);
+	if (!eparam)
+	    return -1;
+	
 	memset(&inreg, 0, sizeof inreg);
 	inreg.eax.b[1] = 0x48;
 	inreg.edx.b[0] = disk;
@@ -153,7 +153,8 @@ int disk_get_params(int disk, struct disk_info *const
diskinfo)
 	diskinfo->lbacnt = diskinfo->cyl * diskinfo->head * diskinfo->spt;
 
 out:
-    lfree(eparam);
+    if (eparam)
+	lfree(eparam);
     return rv;
 }
 
-- 
1.7.2.5

On Fri, 06 Sep, at 01:00:55AM, Raphael S.Carvalho wrote:
> eparam will only be used if EBIOS is available on the underlying disk.
> If not so, then there is no reason to allocate eparam.
> ---
>  com32/lib/syslinux/disk.c |   13 +++++++------
>  1 files changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/com32/lib/syslinux/disk.c b/com32/lib/syslinux/disk.c
> index 093751a..554bed3 100644
> --- a/com32/lib/syslinux/disk.c
> +++ b/com32/lib/syslinux/disk.c
> @@ -73,7 +73,7 @@ int disk_int13_retry(const com32sys_t * inreg, com32sys_t
* outreg)
>  int disk_get_params(int disk, struct disk_info *const diskinfo)
>  {
>      static com32sys_t inreg, outreg;
> -    struct disk_ebios_eparam *eparam;
> +    struct disk_ebios_eparam *eparam = NULL;
>      int rv = 0;
>  
>      memset(diskinfo, 0, sizeof *diskinfo);
> @@ -94,12 +94,12 @@ int disk_get_params(int disk, struct disk_info *const
diskinfo)
>  	diskinfo->ebios = 1;
>      }
>  
> -    eparam = lmalloc(sizeof *eparam);
> -    if (!eparam)
> -	return -1;
> -
>      /* Get extended disk parameters if ebios == 1 */
>      if (diskinfo->ebios) {
> +	eparam = lmalloc(sizeof *eparam);
> +	if (!eparam)
> +	    return -1;
> +	
There's trailing whitespace here.
>  	memset(&inreg, 0, sizeof inreg);
>  	inreg.eax.b[1] = 0x48;
>  	inreg.edx.b[0] = disk;
> @@ -153,7 +153,8 @@ int disk_get_params(int disk, struct disk_info *const
diskinfo)
>  	diskinfo->lbacnt = diskinfo->cyl * diskinfo->head *
diskinfo->spt;
>  
>  out:
> -    lfree(eparam);
> +    if (eparam)
> +	lfree(eparam);
>      return rv;
>  }
You actually don't need to check for 'eparam' being NULL, lfree()
can
handle being called with a NULL pointer.

-- 
Matt Fleming, Intel Open Source Technology Center

> There's trailing whitespace here.
>
>>       memset(&inreg, 0, sizeof inreg);
>>       inreg.eax.b[1] = 0x48;
>>       inreg.edx.b[0] = disk;
>> @@ -153,7 +153,8 @@ int disk_get_params(int disk, struct disk_info
*const diskinfo)
>>       diskinfo->lbacnt = diskinfo->cyl * diskinfo->head *
diskinfo->spt;
>>
>>  out:
>> -    lfree(eparam);
>> +    if (eparam)
>> +     lfree(eparam);
>>      return rv;
>>  }
>
> You actually don't need to check for 'eparam' being NULL,
lfree() can
> handle being called with a NULL pointer.
I will work on these issues, and resend the patch. Also, I definitely
have to fix my text editor to avoid trailing whitespace.

Regards,
Raphael S. Carvalho.