From 10c69bb7d9c492666690f2bd359493c2e900865b Mon Sep 17 00:00:00 2001 From: Prarit Bhargava Date: Tue, 5 May 2015 18:36:17 -0400 Subject: [PATCH] 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 Signed-off-by: Benjamin Romer Signed-off-by: Greg Kroah-Hartman --- .../staging/unisys/visorbus/visorchannel.c | 79 ++++++++----------- .../unisys/visorutil/memregion_direct.c | 4 +- 2 files changed, 36 insertions(+), 47 deletions(-) diff --git a/drivers/staging/unisys/visorbus/visorchannel.c b/drivers/staging/unisys/visorbus/visorchannel.c index 33a4360cf8e6..ff14a0d235f2 100644 --- 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 eb7422fbe20f..6bb439d64511 100644 --- 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); -- 2.39.5