Re: [XenPPC] [PATCH] Fix up potential memory leaks introduced by xencomm patch

2007-02-08 Thread Jimi Xenidis 

mostly Linux Kernel style NITS

On Feb 7, 2007, at 4:26 PM, Jerone Young wrote:


With some of the logic change from the Xencomm patch. In a few
hypercalls introduces a situation where you can potentially have  
memory

leaks if something fails. This patch address these issues.

Signed-off-by: Jerone Young [EMAIL PROTECTED]



diff -r 37ea4cf1281a arch/powerpc/platforms/xen/hcall.c
--- a/arch/powerpc/platforms/xen/hcall.c	Tue Feb 06 17:10:20 2007  
-0500
+++ b/arch/powerpc/platforms/xen/hcall.c	Wed Feb 07 14:50:05 2007  
-0600

@@ -203,11 +203,16 @@ int HYPERVISOR_sched_op(int cmd, void *a
desc = xencomm_map_no_alloc(arg, argsize);
-   if (desc == NULL)
-   return -EINVAL;
-
-   rc = plpar_hcall_norets(XEN_MARK(__HYPERVISOR_sched_op),
-   cmd, desc);
+   if (desc)
+   {


brace on same line


+   rc = plpar_hcall_norets(XEN_MARK(__HYPERVISOR_sched_op),
+   cmd, desc);
+   xencomm_free(desc);
+   }
+   else
+   {


no braces for single line, in fact avoid the single line by pre- 
assigning the rc, the compiler will do the right optimization anyway.



+   rc = -EINVAL;


I thought we agreed that xencomm_map_* failures would be ENOMEM or  
ENOSPC?



+   }
xencomm_free(ports);
@@ -389,8 +394,8 @@ static int xenppc_privcmd_domctl(privcmd
if (copy_to_user(user_op, kern_op, sizeof(xen_domctl_t)))
ret = -EFAULT;
-   xencomm_free(desc);
out:
+   xencomm_free(desc);
xencomm_free(op_desc);
return ret;
}
@@ -463,8 +468,8 @@ static int xenppc_privcmd_sysctl(privcmd
if (copy_to_user(user_op, kern_op, sizeof(xen_sysctl_t)))
ret = -EFAULT;
-   xencomm_free(desc);
out:
+   xencomm_free(desc);
xencomm_free(op_desc);
return ret;
}
@@ -514,8 +519,8 @@ static int xenppc_privcmd_platform_op(pr
if (copy_to_user(user_op, kern_op, sizeof(xen_platform_op_t)))
ret = -EFAULT;
-   xencomm_free(desc);
out:
+   xencomm_free(desc);
xencomm_free(op_desc);
return ret;
}
@@ -547,7 +552,10 @@ int HYPERVISOR_memory_op(unsigned int cm
sizeof(*xen_guest_handle(mop-extent_start)));
if (desc == NULL)
+   {


Curlies again


+   xencomm_free(op_desc);
return -ENOMEM;
+   }
set_xen_guest_handle(mop-extent_start,
 desc);



___
Xen-ppc-devel mailing list
Xen-ppc-devel@lists.xensource.com
http://lists.xensource.com/xen-ppc-devel

Re: [XenPPC] [PATCH] Fix up potential memory leaks introduced by xencomm patch

2007-02-08 Thread Jerone Young 
On Thu, 2007-02-08 at 07:18 -0500, Jimi Xenidis wrote:
 mostly Linux Kernel style NITS
 
 On Feb 7, 2007, at 4:26 PM, Jerone Young wrote:
 
  With some of the logic change from the Xencomm patch. In a few
  hypercalls introduces a situation where you can potentially have  
  memory
  leaks if something fails. This patch address these issues.
 
  Signed-off-by: Jerone Young [EMAIL PROTECTED]
 
  diff -r 37ea4cf1281a arch/powerpc/platforms/xen/hcall.c
  --- a/arch/powerpc/platforms/xen/hcall.cTue Feb 06 17:10:20 2007  
  -0500
  +++ b/arch/powerpc/platforms/xen/hcall.cWed Feb 07 14:50:05 2007  
  -0600
  @@ -203,11 +203,16 @@ int HYPERVISOR_sched_op(int cmd, void *a
  desc = xencomm_map_no_alloc(arg, argsize);
  -   if (desc == NULL)
  -   return -EINVAL;
  -
  -   rc = plpar_hcall_norets(XEN_MARK(__HYPERVISOR_sched_op),
  -   cmd, desc);
  +   if (desc)
  +   {
 
 brace on same line
 
  +   rc = plpar_hcall_norets(XEN_MARK(__HYPERVISOR_sched_op),
  +   cmd, desc);
  +   xencomm_free(desc);
  +   }
  +   else
  +   {
 
 no braces for single line, in fact avoid the single line by pre- 
 assigning the rc, the compiler will do the right optimization anyway.
 
  +   rc = -EINVAL;
 
 I thought we agreed that xencomm_map_* failures would be ENOMEM or  
 ENOSPC?

No changed it (based on your suggestion) to EINVAL. ENOSPC is really
meant for disks out of space. So this actually kind of made the mode
since, it's an invalid argument, because it is too big.

 
  +   }
  xencomm_free(ports);
  @@ -389,8 +394,8 @@ static int xenppc_privcmd_domctl(privcmd
  if (copy_to_user(user_op, kern_op, sizeof(xen_domctl_t)))
  ret = -EFAULT;
  -   xencomm_free(desc);
  out:
  +   xencomm_free(desc);
  xencomm_free(op_desc);
  return ret;
  }
  @@ -463,8 +468,8 @@ static int xenppc_privcmd_sysctl(privcmd
  if (copy_to_user(user_op, kern_op, sizeof(xen_sysctl_t)))
  ret = -EFAULT;
  -   xencomm_free(desc);
  out:
  +   xencomm_free(desc);
  xencomm_free(op_desc);
  return ret;
  }
  @@ -514,8 +519,8 @@ static int xenppc_privcmd_platform_op(pr
  if (copy_to_user(user_op, kern_op, sizeof(xen_platform_op_t)))
  ret = -EFAULT;
  -   xencomm_free(desc);
  out:
  +   xencomm_free(desc);
  xencomm_free(op_desc);
  return ret;
  }
  @@ -547,7 +552,10 @@ int HYPERVISOR_memory_op(unsigned int cm
  sizeof(*xen_guest_handle(mop-extent_start)));
  if (desc == NULL)
  +   {
 
 Curlies again

ok I'll send a new patch with all the curlie braces on the first line.

 
  +   xencomm_free(op_desc);
  return -ENOMEM;
  +   }
  set_xen_guest_handle(mop-extent_start,
   desc);
 


___
Xen-ppc-devel mailing list
Xen-ppc-devel@lists.xensource.com
http://lists.xensource.com/xen-ppc-devel

[XenPPC] [PATCH] Fix up potential memory leaks introduced by xencomm patch

2007-02-07 Thread Jerone Young 
With some of the logic change from the Xencomm patch. In a few
hypercalls introduces a situation where you can potentially have memory
leaks if something fails. This patch address these issues.

Signed-off-by: Jerone Young [EMAIL PROTECTED]
diff -r 37ea4cf1281a arch/powerpc/platforms/xen/hcall.c
--- a/arch/powerpc/platforms/xen/hcall.c	Tue Feb 06 17:10:20 2007 -0500
+++ b/arch/powerpc/platforms/xen/hcall.c	Wed Feb 07 14:50:05 2007 -0600
@@ -203,11 +203,16 @@ int HYPERVISOR_sched_op(int cmd, void *a
 
 	desc = xencomm_map_no_alloc(arg, argsize); 
 
-	if (desc == NULL)
-		return -EINVAL;
-
-	rc = plpar_hcall_norets(XEN_MARK(__HYPERVISOR_sched_op),
-cmd, desc);
+	if (desc)
+	{
+		rc = plpar_hcall_norets(XEN_MARK(__HYPERVISOR_sched_op),
+	cmd, desc);
+		xencomm_free(desc);
+	}
+	else
+	{
+		rc = -EINVAL;
+	}
 
 	xencomm_free(ports);
 
@@ -389,8 +394,8 @@ static int xenppc_privcmd_domctl(privcmd
 	if (copy_to_user(user_op, kern_op, sizeof(xen_domctl_t)))
 		ret = -EFAULT;
 
-	xencomm_free(desc);
 out:
+	xencomm_free(desc);
 	xencomm_free(op_desc);
 	return ret;
 }
@@ -463,8 +468,8 @@ static int xenppc_privcmd_sysctl(privcmd
 	if (copy_to_user(user_op, kern_op, sizeof(xen_sysctl_t)))
 		ret = -EFAULT;
 
-	xencomm_free(desc);
 out:
+	xencomm_free(desc);
 	xencomm_free(op_desc);
 	return ret;
 }
@@ -514,8 +519,8 @@ static int xenppc_privcmd_platform_op(pr
 	if (copy_to_user(user_op, kern_op, sizeof(xen_platform_op_t)))
 		ret = -EFAULT;
 
-	xencomm_free(desc);
 out:
+	xencomm_free(desc);
 	xencomm_free(op_desc);
 	return ret;
 }
@@ -547,7 +552,10 @@ int HYPERVISOR_memory_op(unsigned int cm
 sizeof(*xen_guest_handle(mop-extent_start)));
 
 			if (desc == NULL)
+			{
+xencomm_free(op_desc);
 return -ENOMEM;
+			}
 
 			set_xen_guest_handle(mop-extent_start,
 	 desc);
___
Xen-ppc-devel mailing list
Xen-ppc-devel@lists.xensource.com
http://lists.xensource.com/xen-ppc-devel