staging: unisys: visorchannel cleanup visorchannel_create_guts()

The error handling in this function was broken and while looking at that
I noticed that the whole function was in need of cleanup.  This patch
fixes the error handling, specifically

                if (!p) {
                        visorchannel_destroy(p);
                        channel = NULL;
                }

and does a lot of cleanup.  I also verified that the called functions
returned correct errors, and that led to a change in
visor_memregion_resize(), visorchannel_destroy() and
visor_memregion_destroy().

Signed-off-by: Prarit Bhargava <prarit@redhat.com>
Signed-off-by: Benjamin Romer <benjamin.romer@unisys.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/drivers/staging/unisys/visorbus/visorchannel.c b/drivers/staging/unisys/visorbus/visorchannel.c
index 33a4360cf8e6900a1089d207e9852e773ea15c6c..ff14a0d235f2cbf2b34cba20bf0a2ce7686c6d2a 100644 (file)
--- a/drivers/staging/unisys/visorbus/visorchannel.c
+++ b/drivers/staging/unisys/visorbus/visorchannel.c
@@ -55,60 +55,52 @@ visorchannel_create_guts(HOSTADDRESS physaddr, ulong channel_bytes,
                         struct visorchannel *parent, ulong off, uuid_le guid,
                         BOOL needs_lock)
 {
-       struct visorchannel *p = NULL;
-       void *rc = NULL;
+       struct visorchannel *channel;
+       int err;
+       size_t size = sizeof(struct channel_header);
+       struct memregion *memregion;
 
-       p = kmalloc(sizeof(*p), GFP_KERNEL|__GFP_NORETRY);
-       if (!p) {
-               rc = NULL;
+       channel = kmalloc(sizeof(*channel), GFP_KERNEL|__GFP_NORETRY);
+       if (!channel)
                goto cleanup;
-       }
-       p->memregion = NULL;
-       p->needs_lock = needs_lock;
-       spin_lock_init(&p->insert_lock);
-       spin_lock_init(&p->remove_lock);
+
+       channel->memregion = NULL;
+       channel->needs_lock = needs_lock;
+       spin_lock_init(&channel->insert_lock);
+       spin_lock_init(&channel->remove_lock);
 
        /* prepare chan_hdr (abstraction to read/write channel memory) */
        if (!parent)
-               p->memregion =
-                   visor_memregion_create(physaddr,
-                                          sizeof(struct channel_header));
+               memregion = visor_memregion_create(physaddr, size);
        else
-               p->memregion =
-                   visor_memregion_create_overlapped(parent->memregion,
-                               off, sizeof(struct channel_header));
-       if (!p->memregion) {
-               rc = NULL;
+               memregion = visor_memregion_create_overlapped(parent->memregion,
+                                                             off, size);
+       if (!memregion)
                goto cleanup;
-       }
-       if (visor_memregion_read(p->memregion, 0, &p->chan_hdr,
-                                sizeof(struct channel_header)) < 0) {
-               rc = NULL;
+       channel->memregion = memregion;
+
+       err = visor_memregion_read(channel->memregion, 0, &channel->chan_hdr,
+                                  sizeof(struct channel_header));
+       if (err)
                goto cleanup;
-       }
+
+       /* we had better be a CLIENT of this channel */
        if (channel_bytes == 0)
-               /* we had better be a CLIENT of this channel */
-               channel_bytes = (ulong)p->chan_hdr.size;
+               channel_bytes = (ulong)channel->chan_hdr.size;
        if (uuid_le_cmp(guid, NULL_UUID_LE) == 0)
-               /* we had better be a CLIENT of this channel */
-               guid = p->chan_hdr.chtype;
-       if (visor_memregion_resize(p->memregion, channel_bytes) < 0) {
-               rc = NULL;
+               guid = channel->chan_hdr.chtype;
+
+       err = visor_memregion_resize(channel->memregion, channel_bytes);
+       if (err)
                goto cleanup;
-       }
-       p->size = channel_bytes;
-       p->guid = guid;
 
-       rc = p;
-cleanup:
+       channel->size = channel_bytes;
+       channel->guid = guid;
+       return channel;
 
-       if (!rc) {
-               if (!p) {
-                       visorchannel_destroy(p);
-                       p = NULL;
-               }
-       }
-       return rc;
+cleanup:
+       visorchannel_destroy(channel);
+       return NULL;
 }
 
 struct visorchannel *
@@ -153,10 +145,7 @@ visorchannel_destroy(struct visorchannel *channel)
 {
        if (!channel)
                return;
-       if (channel->memregion) {
-               visor_memregion_destroy(channel->memregion);
-               channel->memregion = NULL;
-       }
+       visor_memregion_destroy(channel->memregion);
        kfree(channel);
 }
 EXPORT_SYMBOL_GPL(visorchannel_destroy);

diff --git a/drivers/staging/unisys/visorutil/memregion_direct.c b/drivers/staging/unisys/visorutil/memregion_direct.c
index eb7422fbe20f4f08db705e8ce6f982450975d750..6bb439d64511cdfd92ab6a1a36c5937530fc898e 100644 (file)
--- a/drivers/staging/unisys/visorutil/memregion_direct.c
+++ b/drivers/staging/unisys/visorutil/memregion_direct.c
@@ -156,7 +156,7 @@ visor_memregion_resize(struct memregion *memregion, ulong newsize)
                unmapit(memregion);
                memregion->nbytes = newsize;
                if (!mapit(memregion))
-                       return -1;
+                       return -EIO;
        }
        return 0;
 }
@@ -197,7 +197,7 @@ EXPORT_SYMBOL_GPL(visor_memregion_write);
 void
 visor_memregion_destroy(struct memregion *memregion)
 {
-       if (memregion == NULL)
+       if (!memregion)
                return;
        if (!memregion->overlapped)
                unmapit(memregion);