All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] staging: unisys: uislib: uislib.c: sparse warning of context imbalance
@ 2014-09-05  9:22 Sudip Mukherjee
  2014-09-08 15:57 ` Romer, Benjamin M
  0 siblings, 1 reply; 16+ messages in thread
From: Sudip Mukherjee @ 2014-09-05  9:22 UTC (permalink / raw)
  To: Benjamin Romer, David Kershner, Greg Kroah-Hartman
  Cc: Sudip Mukherjee, sparmaintainer, devel, linux-kernel

fixed sparse warning : context imbalance in 'pause_device' 
			unexpected unlock
this patch will generate warning from checkpatch for 
lines over 80 character , but since those are user-visible strings
so it was not modified.

Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---

hi , can you please review the patch and see if the approach is correct.
The functiion is still doing the same what it was doing , only the logic
is changed. if the approach is ok, then i can send a patch to fix the
other two similar warning in the file.

 drivers/staging/unisys/uislib/uislib.c | 82 ++++++++++++++++------------------
 1 file changed, 39 insertions(+), 43 deletions(-)

diff --git a/drivers/staging/unisys/uislib/uislib.c b/drivers/staging/unisys/uislib/uislib.c
index 8b89fe3..3a92d9f 100644
--- a/drivers/staging/unisys/uislib/uislib.c
+++ b/drivers/staging/unisys/uislib/uislib.c
@@ -548,6 +548,7 @@ pause_device(CONTROLVM_MESSAGE *msg)
 	struct bus_info *bus;
 	struct device_info *dev;
 	struct guest_msgs cmd;
+	int retval = CONTROLVM_RESP_SUCCESS;
 
 	busNo = msg->cmd.deviceChangeState.busNo;
 	devNo = msg->cmd.deviceChangeState.devNo;
@@ -559,58 +560,53 @@ pause_device(CONTROLVM_MESSAGE *msg)
 			if (devNo >= bus->deviceCount) {
 				LOGERR("CONTROLVM_DEVICE_CHANGESTATE:pause Failed: device(%d) >= deviceCount(%d).",
 				     devNo, bus->deviceCount);
-				read_unlock(&BusListLock);
-				return CONTROLVM_RESP_ERROR_DEVICE_INVALID;
-			}
-			/* make sure this device exists */
-			dev = bus->device[devNo];
-			if (!dev) {
-				LOGERR("CONTROLVM_DEVICE_CHANGESTATE:pause Failed: device %d does not exist.",
-				     devNo);
-				read_unlock(&BusListLock);
-				return CONTROLVM_RESP_ERROR_ALREADY_DONE;
-			}
-			read_unlock(&BusListLock);
-			/* the msg is bound for virtpci; send
-			 * guest_msgs struct to callback
-			 */
-			if (!uuid_le_cmp(dev->channelTypeGuid,
-					UltraVhbaChannelProtocolGuid)) {
-				cmd.msgtype = GUEST_PAUSE_VHBA;
-				cmd.pause_vhba.chanptr = dev->chanptr;
-			} else
-			    if (!uuid_le_cmp(dev->channelTypeGuid,
-					    UltraVnicChannelProtocolGuid)) {
-				cmd.msgtype = GUEST_PAUSE_VNIC;
-				cmd.pause_vnic.chanptr = dev->chanptr;
+				retval = CONTROLVM_RESP_ERROR_DEVICE_INVALID;
 			} else {
-				LOGERR("CONTROLVM_DEVICE_CHANGESTATE:pause Failed: unknown channelTypeGuid.\n");
-				return
-				    CONTROLVM_RESP_ERROR_CHANNEL_TYPE_UNKNOWN;
-			}
-
-			if (!VirtControlChanFunc) {
-				LOGERR("CONTROLVM_DEVICE_CHANGESTATE Failed: virtpci callback not registered.");
-				return
-				    CONTROLVM_RESP_ERROR_VIRTPCI_DRIVER_FAILURE;
-			}
-
-			if (!VirtControlChanFunc(&cmd)) {
-				LOGERR("CONTROLVM_DEVICE_CHANGESTATE:pause Failed: virtpci GUEST_PAUSE_[VHBA||VNIC] returned error.");
-				return CONTROLVM_RESP_ERROR_VIRTPCI_DRIVER_CALLBACK_ERROR;
+				/* make sure this device exists */
+				dev = bus->device[devNo];
+				if (!dev) {
+					LOGERR("CONTROLVM_DEVICE_CHANGESTATE:pause Failed: device %d does not exist.",
+					     devNo);
+					retval =
+					  CONTROLVM_RESP_ERROR_ALREADY_DONE;
+				}
 			}
 			break;
 		}
 	}
-
 	if (!bus) {
 		LOGERR("CONTROLVM_DEVICE_CHANGESTATE:pause Failed: bus %d does not exist",
 		     busNo);
-		read_unlock(&BusListLock);
-		return CONTROLVM_RESP_ERROR_BUS_INVALID;
+		retval = CONTROLVM_RESP_ERROR_BUS_INVALID;
 	}
-
-	return CONTROLVM_RESP_SUCCESS;
+	read_unlock(&BusListLock);
+	if (retval == CONTROLVM_RESP_SUCCESS) {
+		/* the msg is bound for virtpci; send
+		 * guest_msgs struct to callback
+		 */
+		if (!uuid_le_cmp(dev->channelTypeGuid,
+				UltraVhbaChannelProtocolGuid)) {
+			cmd.msgtype = GUEST_PAUSE_VHBA;
+			cmd.pause_vhba.chanptr = dev->chanptr;
+		} else if (!uuid_le_cmp(dev->channelTypeGuid,
+					UltraVnicChannelProtocolGuid)) {
+			cmd.msgtype = GUEST_PAUSE_VNIC;
+			cmd.pause_vnic.chanptr = dev->chanptr;
+		} else {
+			LOGERR("CONTROLVM_DEVICE_CHANGESTATE:pause Failed: unknown channelTypeGuid.\n");
+			return CONTROLVM_RESP_ERROR_CHANNEL_TYPE_UNKNOWN;
+		}
+		if (!VirtControlChanFunc) {
+			LOGERR("CONTROLVM_DEVICE_CHANGESTATE Failed: virtpci callback not registered.");
+			return CONTROLVM_RESP_ERROR_VIRTPCI_DRIVER_FAILURE;
+		}
+		if (!VirtControlChanFunc(&cmd)) {
+			LOGERR("CONTROLVM_DEVICE_CHANGESTATE:pause Failed: virtpci GUEST_PAUSE_[VHBA||VNIC] returned error.");
+			return
+			  CONTROLVM_RESP_ERROR_VIRTPCI_DRIVER_CALLBACK_ERROR;
+		}
+	}
+	return retval;
 }
 
 static int
-- 
1.8.1.2


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH] staging: unisys: uislib: uislib.c: sparse warning of context imbalance
  2014-09-05  9:22 [PATCH] staging: unisys: uislib: uislib.c: sparse warning of context imbalance Sudip Mukherjee
@ 2014-09-08 15:57 ` Romer, Benjamin M
  2014-09-08 16:27   ` Sudip Mukherjee
  2014-09-08 17:06   ` Greg Kroah-Hartman
  0 siblings, 2 replies; 16+ messages in thread
From: Romer, Benjamin M @ 2014-09-08 15:57 UTC (permalink / raw)
  To: Sudip Mukherjee
  Cc: Kershner, David A, *S-Par-Maintainer, Greg Kroah-Hartman, devel,
	linux-kernel

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 1055 bytes --]

On Fri, 2014-09-05 at 14:52 +0530, Sudip Mukherjee wrote:
> fixed sparse warning : context imbalance in 'pause_device' 
> 			unexpected unlock
> this patch will generate warning from checkpatch for 
> lines over 80 character , but since those are user-visible strings
> so it was not modified.
> 
> Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
> ---
> 
> hi , can you please review the patch and see if the approach is correct.
> The functiion is still doing the same what it was doing , only the logic
> is changed. if the approach is ok, then i can send a patch to fix the
> other two similar warning in the file.

Hi Sudip,

I was able to successfully build and test your patch. The changes look
good to me too, so I think we should take this patch. :)

Thanks!
-- Ben

Signed-off-by: Benjamin Romer <benjamin.romer@unisys.com>
Tested-by: Benjamin Romer <benjamin.romer@unisys.com>
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] staging: unisys: uislib: uislib.c: sparse warning of context imbalance
  2014-09-08 15:57 ` Romer, Benjamin M
@ 2014-09-08 16:27   ` Sudip Mukherjee
  2014-09-08 16:30     ` Romer, Benjamin M
  2014-09-08 17:06   ` Greg Kroah-Hartman
  1 sibling, 1 reply; 16+ messages in thread
From: Sudip Mukherjee @ 2014-09-08 16:27 UTC (permalink / raw)
  To: Romer, Benjamin M
  Cc: Kershner, David A, *S-Par-Maintainer, Greg Kroah-Hartman, devel,
	linux-kernel

On Mon, Sep 08, 2014 at 10:57:08AM -0500, Romer, Benjamin M wrote:
> On Fri, 2014-09-05 at 14:52 +0530, Sudip Mukherjee wrote:
> > fixed sparse warning : context imbalance in 'pause_device' 
> > 			unexpected unlock
> > this patch will generate warning from checkpatch for 
> > lines over 80 character , but since those are user-visible strings
> > so it was not modified.
> > 
> > Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
> > ---
> > 
> > hi , can you please review the patch and see if the approach is correct.
> > The functiion is still doing the same what it was doing , only the logic
> > is changed. if the approach is ok, then i can send a patch to fix the
> > other two similar warning in the file.
> 
> Hi Sudip,
> 
> I was able to successfully build and test your patch. The changes look
> good to me too, so I think we should take this patch. :)
> 
> Thanks!
> -- Ben
> 
Hi Ben,
thanks. the same file is having two more similar warnings. if you want i can
resend a patch fixing all the three warnings , or i can send two separate
patches. 
I personally will prefer two separate patches , as that will be
easier for you to test and review.

thanks
sudip

> Signed-off-by: Benjamin Romer <benjamin.romer@unisys.com>
> Tested-by: Benjamin Romer <benjamin.romer@unisys.com>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] staging: unisys: uislib: uislib.c: sparse warning of context imbalance
  2014-09-08 16:27   ` Sudip Mukherjee
@ 2014-09-08 16:30     ` Romer, Benjamin M
  2014-09-08 16:38       ` Sudip Mukherjee
  0 siblings, 1 reply; 16+ messages in thread
From: Romer, Benjamin M @ 2014-09-08 16:30 UTC (permalink / raw)
  To: Sudip Mukherjee
  Cc: Kershner, David A, *S-Par-Maintainer, Greg Kroah-Hartman, devel,
	linux-kernel

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 561 bytes --]

On Mon, 2014-09-08 at 21:57 +0530, Sudip Mukherjee wrote:
> Hi Ben,
> thanks. the same file is having two more similar warnings. if you want i can
> resend a patch fixing all the three warnings , or i can send two separate
> patches. 
> I personally will prefer two separate patches , as that will be
> easier for you to test and review.
> 
> thanks
> sudip

That would be perfect, thanks. :)

-- Ben
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] staging: unisys: uislib: uislib.c: sparse warning of context imbalance
  2014-09-08 16:30     ` Romer, Benjamin M
@ 2014-09-08 16:38       ` Sudip Mukherjee
  2014-09-08 16:48         ` Romer, Benjamin M
  0 siblings, 1 reply; 16+ messages in thread
From: Sudip Mukherjee @ 2014-09-08 16:38 UTC (permalink / raw)
  To: Romer, Benjamin M
  Cc: Kershner, David A, *S-Par-Maintainer, Greg Kroah-Hartman, devel,
	linux-kernel

On Mon, Sep 08, 2014 at 11:30:48AM -0500, Romer, Benjamin M wrote:
> On Mon, 2014-09-08 at 21:57 +0530, Sudip Mukherjee wrote:
> > Hi Ben,
> > thanks. the same file is having two more similar warnings. if you want i can
> > resend a patch fixing all the three warnings , or i can send two separate
> > patches. 
> > I personally will prefer two separate patches , as that will be
> > easier for you to test and review.
> > 
> > thanks
> > sudip
> 
> That would be perfect, thanks. :)
> 
> -- Ben

Hi Ben,
sorry to disturb you again. i got confused , which one is perfect one combined patch or 
separate patches?

thanks
sudip

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] staging: unisys: uislib: uislib.c: sparse warning of context imbalance
  2014-09-08 16:38       ` Sudip Mukherjee
@ 2014-09-08 16:48         ` Romer, Benjamin M
  2014-09-08 17:02           ` Sudip Mukherjee
  0 siblings, 1 reply; 16+ messages in thread
From: Romer, Benjamin M @ 2014-09-08 16:48 UTC (permalink / raw)
  To: Sudip Mukherjee
  Cc: Kershner, David A, *S-Par-Maintainer, Greg Kroah-Hartman, devel,
	linux-kernel

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 401 bytes --]

On Mon, 2014-09-08 at 22:08 +0530, Sudip Mukherjee wrote:
> Hi Ben,
> sorry to disturb you again. i got confused , which one is perfect one combined patch or 
> separate patches?
> 
> thanks
> sudip

Two patches, as you preferred.

-- Ben


ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] staging: unisys: uislib: uislib.c: sparse warning of context imbalance
  2014-09-08 16:48         ` Romer, Benjamin M
@ 2014-09-08 17:02           ` Sudip Mukherjee
  0 siblings, 0 replies; 16+ messages in thread
From: Sudip Mukherjee @ 2014-09-08 17:02 UTC (permalink / raw)
  To: Romer, Benjamin M
  Cc: Kershner, David A, *S-Par-Maintainer, Greg Kroah-Hartman, devel,
	linux-kernel

On Mon, Sep 08, 2014 at 11:48:44AM -0500, Romer, Benjamin M wrote:
> On Mon, 2014-09-08 at 22:08 +0530, Sudip Mukherjee wrote:
> > Hi Ben,
> > sorry to disturb you again. i got confused , which one is perfect one combined patch or 
> > separate patches?
> > 
> > thanks
> > sudip
> 
> Two patches, as you preferred.
> 
> -- Ben
> 
> 
ok. and if you get time can you please also have a look at :
[PATCH v4] staging: unisys: uislib: uisqueue.c: rewrite of do_locked_client_insert
Dan Carpenter has already reviwed it.

thanks
sudip


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] staging: unisys: uislib: uislib.c: sparse warning of context imbalance
  2014-09-08 15:57 ` Romer, Benjamin M
  2014-09-08 16:27   ` Sudip Mukherjee
@ 2014-09-08 17:06   ` Greg Kroah-Hartman
  2014-09-08 17:10     ` Romer, Benjamin M
  1 sibling, 1 reply; 16+ messages in thread
From: Greg Kroah-Hartman @ 2014-09-08 17:06 UTC (permalink / raw)
  To: Romer, Benjamin M
  Cc: Sudip Mukherjee, devel, *S-Par-Maintainer, Kershner, David A,
	linux-kernel

On Mon, Sep 08, 2014 at 10:57:08AM -0500, Romer, Benjamin M wrote:
> On Fri, 2014-09-05 at 14:52 +0530, Sudip Mukherjee wrote:
> > fixed sparse warning : context imbalance in 'pause_device' 
> > 			unexpected unlock
> > this patch will generate warning from checkpatch for 
> > lines over 80 character , but since those are user-visible strings
> > so it was not modified.
> > 
> > Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
> > ---
> > 
> > hi , can you please review the patch and see if the approach is correct.
> > The functiion is still doing the same what it was doing , only the logic
> > is changed. if the approach is ok, then i can send a patch to fix the
> > other two similar warning in the file.
> 
> Hi Sudip,
> 
> I was able to successfully build and test your patch. The changes look
> good to me too, so I think we should take this patch. :)

Traditionally, you would respond with a:
	Acked-by: Developer Name <email@address>
so I can add it to the patch.

Care to do that here?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] staging: unisys: uislib: uislib.c: sparse warning of context imbalance
  2014-09-08 17:06   ` Greg Kroah-Hartman
@ 2014-09-08 17:10     ` Romer, Benjamin M
  2014-09-08 17:24       ` Greg Kroah-Hartman
  0 siblings, 1 reply; 16+ messages in thread
From: Romer, Benjamin M @ 2014-09-08 17:10 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: *S-Par-Maintainer, Kershner, David A, Sudip Mukherjee, devel,
	linux-kernel

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 544 bytes --]

On Mon, 2014-09-08 at 10:06 -0700, Greg Kroah-Hartman wrote:
> Traditionally, you would respond with a:
> 	Acked-by: Developer Name <email@address>
> so I can add it to the patch.
> 
> Care to do that here?
> 
> thanks,
> 
> greg k-h

Of course. :)

Acked-by: Benjamin Romer <benjamin.romer@unisys.com>

Should I do this instead of Tested-by and Signed-off-by, in the future?

-- Ben
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] staging: unisys: uislib: uislib.c: sparse warning of context imbalance
  2014-09-08 17:10     ` Romer, Benjamin M
@ 2014-09-08 17:24       ` Greg Kroah-Hartman
  2014-09-08 17:33         ` Romer, Benjamin M
  0 siblings, 1 reply; 16+ messages in thread
From: Greg Kroah-Hartman @ 2014-09-08 17:24 UTC (permalink / raw)
  To: Romer, Benjamin M
  Cc: devel, Kershner, David A, *S-Par-Maintainer, Sudip Mukherjee,
	linux-kernel

On Mon, Sep 08, 2014 at 12:10:55PM -0500, Romer, Benjamin M wrote:
> On Mon, 2014-09-08 at 10:06 -0700, Greg Kroah-Hartman wrote:
> > Traditionally, you would respond with a:
> > 	Acked-by: Developer Name <email@address>
> > so I can add it to the patch.
> > 
> > Care to do that here?
> > 
> > thanks,
> > 
> > greg k-h
> 
> Of course. :)
> 
> Acked-by: Benjamin Romer <benjamin.romer@unisys.com>
> 
> Should I do this instead of Tested-by and Signed-off-by, in the future?

If you test it, do a tested-by.

If you sign off on it (i.e. it flows through you to me), then a
signed-off is correct.

Hope this helps,

greg k-h

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] staging: unisys: uislib: uislib.c: sparse warning of context imbalance
  2014-09-08 17:24       ` Greg Kroah-Hartman
@ 2014-09-08 17:33         ` Romer, Benjamin M
  0 siblings, 0 replies; 16+ messages in thread
From: Romer, Benjamin M @ 2014-09-08 17:33 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Kershner, David A, *S-Par-Maintainer, devel, Sudip Mukherjee,
	linux-kernel

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 554 bytes --]

On Mon, 2014-09-08 at 10:24 -0700, Greg Kroah-Hartman wrote:
> If you test it, do a tested-by.
> 
> If you sign off on it (i.e. it flows through you to me), then a
> signed-off is correct.
> 
> Hope this helps,
> 
> greg k-h

It does. :) We should add the Tested-by then, too.

Acked-by: Benjamin Romer <benjamin.romer@unisys.com>
Tested-by: Benjamin Romer <benjamin.romer@unisys.com>

-- Ben

ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] staging: unisys: uislib: uislib.c: sparse warning of context imbalance
  2014-09-12 12:27 Sudip Mukherjee
@ 2014-09-12 18:20 ` Ben Romer
  0 siblings, 0 replies; 16+ messages in thread
From: Ben Romer @ 2014-09-12 18:20 UTC (permalink / raw)
  To: sudipm.mukherjee, David.Kershner, Benjamin.Romer
  Cc: sudipm.mukherjee, SParMaintainer, linux-kernel, gregkh, devel

Sudip Mukherjee <sudipm.mukherjee@gmail.com> wrote:

> fixed sparse warning : context imbalance in 'destroy_device'
>                         unexpected unlock
> this patch will generate warning from checkpatch for
> lines over 80 character , but since those are user-visible strings
> so it was not modified.
>
> Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>

I tested this patch and it looks good also. Thanks! :)

-- Ben

Tested-by: Benjamin Romer <benjamin.romer@unisys.com>
Acked-by: Benjamin Romer <benjamin.romer@unisys.com>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH] staging: unisys: uislib: uislib.c: sparse warning of context imbalance
@ 2014-09-12 12:27 Sudip Mukherjee
  2014-09-12 18:20 ` Ben Romer
  0 siblings, 1 reply; 16+ messages in thread
From: Sudip Mukherjee @ 2014-09-12 12:27 UTC (permalink / raw)
  To: Benjamin Romer, David Kershner
  Cc: Sudip Mukherjee, Greg Kroah-Hartman, sparmaintainer, devel, linux-kernel

fixed sparse warning : context imbalance in 'destroy_device'
                        unexpected unlock
this patch will generate warning from checkpatch for
lines over 80 character , but since those are user-visible strings
so it was not modified.

Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---
 drivers/staging/unisys/uislib/uislib.c | 117 ++++++++++++++++-----------------
 1 file changed, 58 insertions(+), 59 deletions(-)

diff --git a/drivers/staging/unisys/uislib/uislib.c b/drivers/staging/unisys/uislib/uislib.c
index ae923c3..1823f6f 100644
--- a/drivers/staging/unisys/uislib/uislib.c
+++ b/drivers/staging/unisys/uislib/uislib.c
@@ -685,6 +685,7 @@ destroy_device(CONTROLVM_MESSAGE *msg, char *buf)
 	struct bus_info *bus;
 	struct device_info *dev;
 	struct guest_msgs cmd;
+	int retval = CONTROLVM_RESP_SUCCESS;
 
 	busNo = msg->cmd.destroyDevice.busNo;
 	devNo = msg->cmd.destroyDevice.devNo;
@@ -696,63 +697,18 @@ destroy_device(CONTROLVM_MESSAGE *msg, char *buf)
 			/* make sure the device number is valid */
 			if (devNo >= bus->deviceCount) {
 				LOGERR("CONTROLVM_DEVICE_DESTORY Failed: device(%d) >= deviceCount(%d).",
-				     devNo, bus->deviceCount);
-				read_unlock(&BusListLock);
-				return CONTROLVM_RESP_ERROR_DEVICE_INVALID;
-			}
-			/* make sure this device exists */
-			dev = bus->device[devNo];
-			if (!dev) {
-				LOGERR("CONTROLVM_DEVICE_DESTROY Failed: device %d does not exist.",
-				     devNo);
-				read_unlock(&BusListLock);
-				return CONTROLVM_RESP_ERROR_ALREADY_DONE;
-			}
-			read_unlock(&BusListLock);
-			/* the msg is bound for virtpci; send
-			 * guest_msgs struct to callback
-			 */
-			if (!uuid_le_cmp(dev->channelTypeGuid,
-					UltraVhbaChannelProtocolGuid)) {
-				cmd.msgtype = GUEST_DEL_VHBA;
-				cmd.del_vhba.chanptr = dev->chanptr;
-			} else
-			    if (!uuid_le_cmp(dev->channelTypeGuid,
-					    UltraVnicChannelProtocolGuid)) {
-				cmd.msgtype = GUEST_DEL_VNIC;
-				cmd.del_vnic.chanptr = dev->chanptr;
+				       devNo, bus->deviceCount);
+				retval = CONTROLVM_RESP_ERROR_DEVICE_INVALID;
 			} else {
-				LOGERR("CONTROLVM_DEVICE_DESTROY Failed: unknown channelTypeGuid.\n");
-				return
-				    CONTROLVM_RESP_ERROR_CHANNEL_TYPE_UNKNOWN;
-			}
-
-			if (!VirtControlChanFunc) {
-				LOGERR("CONTROLVM_DEVICE_DESTORY Failed: virtpci callback not registered.");
-				return
-				    CONTROLVM_RESP_ERROR_VIRTPCI_DRIVER_FAILURE;
-			}
-
-			if (!VirtControlChanFunc(&cmd)) {
-				LOGERR("CONTROLVM_DEVICE_DESTROY Failed: virtpci GUEST_DEL_[VHBA||VNIC] returned error.");
-				return CONTROLVM_RESP_ERROR_VIRTPCI_DRIVER_CALLBACK_ERROR;
-			}
-/* you must disable channel interrupts BEFORE you unmap the channel,
- * because if you unmap first, there may still be some activity going
- * on which accesses the channel and you will get a "unable to handle
- * kernel paging request"
- */
-			if (dev->polling) {
-				LOGINF("calling uislib_disable_channel_interrupts");
-				uislib_disable_channel_interrupts(busNo, devNo);
-			}
-			/* unmap the channel memory for the device. */
-			if (!msg->hdr.Flags.testMessage) {
-				LOGINF("destroy_device, doing iounmap");
-				uislib_iounmap(dev->chanptr);
+				/* make sure this device exists */
+				dev = bus->device[devNo];
+				if (!dev) {
+					LOGERR("CONTROLVM_DEVICE_DESTROY Failed: device %d does not exist.",
+					       devNo);
+					retval =
+					     CONTROLVM_RESP_ERROR_ALREADY_DONE;
+				}
 			}
-			kfree(dev);
-			bus->device[devNo] = NULL;
 			break;
 		}
 	}
@@ -760,11 +716,54 @@ destroy_device(CONTROLVM_MESSAGE *msg, char *buf)
 	if (!bus) {
 		LOGERR("CONTROLVM_DEVICE_DESTROY Failed: bus %d does not exist",
 		       busNo);
-		read_unlock(&BusListLock);
-		return CONTROLVM_RESP_ERROR_BUS_INVALID;
+		retval = CONTROLVM_RESP_ERROR_BUS_INVALID;
 	}
-
-	return CONTROLVM_RESP_SUCCESS;
+	read_unlock(&BusListLock);
+	if (retval == CONTROLVM_RESP_SUCCESS) {
+		/* the msg is bound for virtpci; send
+		 * guest_msgs struct to callback
+		 */
+		if (!uuid_le_cmp(dev->channelTypeGuid,
+				 UltraVhbaChannelProtocolGuid)) {
+			cmd.msgtype = GUEST_DEL_VHBA;
+			cmd.del_vhba.chanptr = dev->chanptr;
+		} else if (!uuid_le_cmp(dev->channelTypeGuid,
+					UltraVnicChannelProtocolGuid)) {
+			cmd.msgtype = GUEST_DEL_VNIC;
+			cmd.del_vnic.chanptr = dev->chanptr;
+		} else {
+			LOGERR("CONTROLVM_DEVICE_DESTROY Failed: unknown channelTypeGuid.\n");
+			return
+			    CONTROLVM_RESP_ERROR_CHANNEL_TYPE_UNKNOWN;
+		}
+		if (!VirtControlChanFunc) {
+			LOGERR("CONTROLVM_DEVICE_DESTORY Failed: virtpci callback not registered.");
+			return
+			    CONTROLVM_RESP_ERROR_VIRTPCI_DRIVER_FAILURE;
+		}
+		if (!VirtControlChanFunc(&cmd)) {
+			LOGERR("CONTROLVM_DEVICE_DESTROY Failed: virtpci GUEST_DEL_[VHBA||VNIC] returned error.");
+			return
+			    CONTROLVM_RESP_ERROR_VIRTPCI_DRIVER_CALLBACK_ERROR;
+		}
+/* you must disable channel interrupts BEFORE you unmap the channel,
+ * because if you unmap first, there may still be some activity going
+ * on which accesses the channel and you will get a "unable to handle
+ * kernel paging request"
+ */
+		if (dev->polling) {
+			LOGINF("calling uislib_disable_channel_interrupts");
+			uislib_disable_channel_interrupts(busNo, devNo);
+		}
+		/* unmap the channel memory for the device. */
+		if (!msg->hdr.Flags.testMessage) {
+			LOGINF("destroy_device, doing iounmap");
+			uislib_iounmap(dev->chanptr);
+		}
+		kfree(dev);
+		bus->device[devNo] = NULL;
+	}
+	return retval;
 }
 
 static int
-- 
1.8.1.2


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH] staging: unisys: uislib: uislib.c: sparse warning of context imbalance
  2014-09-10 13:29 ` Romer, Benjamin M
@ 2014-09-10 13:55   ` Sudip Mukherjee
  0 siblings, 0 replies; 16+ messages in thread
From: Sudip Mukherjee @ 2014-09-10 13:55 UTC (permalink / raw)
  To: Romer, Benjamin M
  Cc: Kershner, David A, *S-Par-Maintainer, Greg Kroah-Hartman, devel,
	linux-kernel

On Wed, Sep 10, 2014 at 08:29:26AM -0500, Romer, Benjamin M wrote:
> On Tue, 2014-09-09 at 16:11 +0530, Sudip Mukherjee wrote:
> > fixed sparse warning : context imbalance in 'resume_device'
> >                         unexpected unlock
> > this patch will generate warning from checkpatch for
> > lines over 80 character , but since those are user-visible strings
> > so it was not modified.
> > 
> > Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
> 
> I tested your changes, and this patch works fine. Thanks!
> 
> -- Ben
> 
> Acked-by: Benjamin Romer <benjamin.romer@unisys.com>
> Tested-by: Benjamin Romer <benjamin.romer@unisys.com>

thanks,
so after Greg K-H applies it then the patch to remove the last warning from this file.

thanks
sudip

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] staging: unisys: uislib: uislib.c: sparse warning of context imbalance
  2014-09-09 10:41 Sudip Mukherjee
@ 2014-09-10 13:29 ` Romer, Benjamin M
  2014-09-10 13:55   ` Sudip Mukherjee
  0 siblings, 1 reply; 16+ messages in thread
From: Romer, Benjamin M @ 2014-09-10 13:29 UTC (permalink / raw)
  To: Sudip Mukherjee
  Cc: Kershner, David A, *S-Par-Maintainer, Greg Kroah-Hartman, devel,
	linux-kernel

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 709 bytes --]

On Tue, 2014-09-09 at 16:11 +0530, Sudip Mukherjee wrote:
> fixed sparse warning : context imbalance in 'resume_device'
>                         unexpected unlock
> this patch will generate warning from checkpatch for
> lines over 80 character , but since those are user-visible strings
> so it was not modified.
> 
> Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>

I tested your changes, and this patch works fine. Thanks!

-- Ben

Acked-by: Benjamin Romer <benjamin.romer@unisys.com>
Tested-by: Benjamin Romer <benjamin.romer@unisys.com>
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH] staging: unisys: uislib: uislib.c: sparse warning of context imbalance
@ 2014-09-09 10:41 Sudip Mukherjee
  2014-09-10 13:29 ` Romer, Benjamin M
  0 siblings, 1 reply; 16+ messages in thread
From: Sudip Mukherjee @ 2014-09-09 10:41 UTC (permalink / raw)
  To: Benjamin Romer, David Kershner, Greg Kroah-Hartman
  Cc: Sudip Mukherjee, sparmaintainer, devel, linux-kernel

fixed sparse warning : context imbalance in 'resume_device'
                        unexpected unlock
this patch will generate warning from checkpatch for
lines over 80 character , but since those are user-visible strings
so it was not modified.

Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---
 drivers/staging/unisys/uislib/uislib.c | 81 ++++++++++++++++------------------
 1 file changed, 39 insertions(+), 42 deletions(-)

diff --git a/drivers/staging/unisys/uislib/uislib.c b/drivers/staging/unisys/uislib/uislib.c
index c4d6cae..ae923c3 100644
--- a/drivers/staging/unisys/uislib/uislib.c
+++ b/drivers/staging/unisys/uislib/uislib.c
@@ -616,6 +616,7 @@ resume_device(CONTROLVM_MESSAGE *msg)
 	struct bus_info *bus;
 	struct device_info *dev;
 	struct guest_msgs cmd;
+	int retval = CONTROLVM_RESP_SUCCESS;
 
 	busNo = msg->cmd.deviceChangeState.busNo;
 	devNo = msg->cmd.deviceChangeState.devNo;
@@ -627,45 +628,16 @@ resume_device(CONTROLVM_MESSAGE *msg)
 			if (devNo >= bus->deviceCount) {
 				LOGERR("CONTROLVM_DEVICE_CHANGESTATE:resume Failed: device(%d) >= deviceCount(%d).",
 				     devNo, bus->deviceCount);
-				read_unlock(&BusListLock);
-				return CONTROLVM_RESP_ERROR_DEVICE_INVALID;
-			}
-			/* make sure this device exists */
-			dev = bus->device[devNo];
-			if (!dev) {
-				LOGERR("CONTROLVM_DEVICE_CHANGESTATE:resume Failed: device %d does not exist.",
-				     devNo);
-				read_unlock(&BusListLock);
-				return CONTROLVM_RESP_ERROR_ALREADY_DONE;
-			}
-			read_unlock(&BusListLock);
-			/* the msg is bound for virtpci; send
-			 * guest_msgs struct to callback
-			 */
-			if (!uuid_le_cmp(dev->channelTypeGuid,
-					UltraVhbaChannelProtocolGuid)) {
-				cmd.msgtype = GUEST_RESUME_VHBA;
-				cmd.resume_vhba.chanptr = dev->chanptr;
-			} else
-			    if (!uuid_le_cmp(dev->channelTypeGuid,
-					    UltraVnicChannelProtocolGuid)) {
-				cmd.msgtype = GUEST_RESUME_VNIC;
-				cmd.resume_vnic.chanptr = dev->chanptr;
+				retval = CONTROLVM_RESP_ERROR_DEVICE_INVALID;
 			} else {
-				LOGERR("CONTROLVM_DEVICE_CHANGESTATE:resume Failed: unknown channelTypeGuid.\n");
-				return
-				    CONTROLVM_RESP_ERROR_CHANNEL_TYPE_UNKNOWN;
-			}
-
-			if (!VirtControlChanFunc) {
-				LOGERR("CONTROLVM_DEVICE_CHANGESTATE Failed: virtpci callback not registered.");
-				return
-				    CONTROLVM_RESP_ERROR_VIRTPCI_DRIVER_FAILURE;
-			}
-
-			if (!VirtControlChanFunc(&cmd)) {
-				LOGERR("CONTROLVM_DEVICE_CHANGESTATE:resume Failed: virtpci GUEST_RESUME_[VHBA||VNIC] returned error.");
-				return CONTROLVM_RESP_ERROR_VIRTPCI_DRIVER_CALLBACK_ERROR;
+				/* make sure this device exists */
+				dev = bus->device[devNo];
+				if (!dev) {
+					LOGERR("CONTROLVM_DEVICE_CHANGESTATE:resume Failed: device %d does not exist.",
+					     devNo);
+					retval =
+					  CONTROLVM_RESP_ERROR_ALREADY_DONE;
+				}
 			}
 			break;
 		}
@@ -674,11 +646,36 @@ resume_device(CONTROLVM_MESSAGE *msg)
 	if (!bus) {
 		LOGERR("CONTROLVM_DEVICE_CHANGESTATE:resume Failed: bus %d does not exist",
 		     busNo);
-		read_unlock(&BusListLock);
-		return CONTROLVM_RESP_ERROR_BUS_INVALID;
+		retval = CONTROLVM_RESP_ERROR_BUS_INVALID;
 	}
-
-	return CONTROLVM_RESP_SUCCESS;
+	read_unlock(&BusListLock);
+	/* the msg is bound for virtpci; send
+	 * guest_msgs struct to callback
+	 */
+	if (retval == CONTROLVM_RESP_SUCCESS) {
+		if (!uuid_le_cmp(dev->channelTypeGuid,
+				 UltraVhbaChannelProtocolGuid)) {
+			cmd.msgtype = GUEST_RESUME_VHBA;
+			cmd.resume_vhba.chanptr = dev->chanptr;
+		} else if (!uuid_le_cmp(dev->channelTypeGuid,
+					UltraVnicChannelProtocolGuid)) {
+			cmd.msgtype = GUEST_RESUME_VNIC;
+			cmd.resume_vnic.chanptr = dev->chanptr;
+		} else {
+			LOGERR("CONTROLVM_DEVICE_CHANGESTATE:resume Failed: unknown channelTypeGuid.\n");
+			return CONTROLVM_RESP_ERROR_CHANNEL_TYPE_UNKNOWN;
+		}
+		if (!VirtControlChanFunc) {
+			LOGERR("CONTROLVM_DEVICE_CHANGESTATE Failed: virtpci callback not registered.");
+			return CONTROLVM_RESP_ERROR_VIRTPCI_DRIVER_FAILURE;
+		}
+		if (!VirtControlChanFunc(&cmd)) {
+			LOGERR("CONTROLVM_DEVICE_CHANGESTATE:resume Failed: virtpci GUEST_RESUME_[VHBA||VNIC] returned error.");
+			return
+			  CONTROLVM_RESP_ERROR_VIRTPCI_DRIVER_CALLBACK_ERROR;
+		}
+	}
+	return retval;
 }
 
 static int
-- 
1.8.1.2


^ permalink raw reply related	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2014-09-12 18:21 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-09-05  9:22 [PATCH] staging: unisys: uislib: uislib.c: sparse warning of context imbalance Sudip Mukherjee
2014-09-08 15:57 ` Romer, Benjamin M
2014-09-08 16:27   ` Sudip Mukherjee
2014-09-08 16:30     ` Romer, Benjamin M
2014-09-08 16:38       ` Sudip Mukherjee
2014-09-08 16:48         ` Romer, Benjamin M
2014-09-08 17:02           ` Sudip Mukherjee
2014-09-08 17:06   ` Greg Kroah-Hartman
2014-09-08 17:10     ` Romer, Benjamin M
2014-09-08 17:24       ` Greg Kroah-Hartman
2014-09-08 17:33         ` Romer, Benjamin M
2014-09-09 10:41 Sudip Mukherjee
2014-09-10 13:29 ` Romer, Benjamin M
2014-09-10 13:55   ` Sudip Mukherjee
2014-09-12 12:27 Sudip Mukherjee
2014-09-12 18:20 ` Ben Romer

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.