* [PATCH 1/1] staging: unisys: parser.c bug
@ 2014-11-17 20:06 Jeffrey Brown
0 siblings, 0 replies; 4+ messages in thread
From: Jeffrey Brown @ 2014-11-17 20:06 UTC (permalink / raw)
To: gregkh; +Cc: jkc, driverdev-devel, sparmaintainer, Jeffrey Brown
Replaced cleanups: with err_rgn, err_ctx, and out_rgn in the file.
This is to have proper error and success handling. I also removed
the variable rc because like cleanups, rc is redundant in this file
Signed-off-by: Jeffrey Brown <Jeffrey.Brown@unisys.com>
---
drivers/staging/unisys/visorchipset/parser.c | 62 +++++++++++-----------------
1 file changed, 25 insertions(+), 37 deletions(-)
diff --git a/drivers/staging/unisys/visorchipset/parser.c b/drivers/staging/unisys/visorchipset/parser.c
index 5f6a7b2..e4e230f 100644
--- a/drivers/staging/unisys/visorchipset/parser.c
+++ b/drivers/staging/unisys/visorchipset/parser.c
@@ -45,7 +45,6 @@ parser_init_guts(u64 addr, u32 bytes, BOOL is_local,
BOOL has_standard_payload_header, BOOL *try_again)
{
int allocbytes = sizeof(struct parser_context_tag) + bytes;
- struct parser_context_tag *rc = NULL;
struct parser_context_tag *ctx = NULL;
struct memregion *rgn = NULL;
struct spar_controlvm_parameters_header *phdr = NULL;
@@ -64,8 +63,7 @@ parser_init_guts(u64 addr, u32 bytes, BOOL is_local,
MAX_CONTROLVM_PAYLOAD_BYTES);
if (try_again)
*try_again = TRUE;
- rc = NULL;
- goto cleanups;
+ return NULL;
}
ctx = kzalloc(allocbytes, GFP_KERNEL|__GFP_NORETRY);
if (ctx == NULL) {
@@ -73,8 +71,7 @@ parser_init_guts(u64 addr, u32 bytes, BOOL is_local,
__func__, __FILE__, __LINE__, allocbytes);
if (try_again)
*try_again = TRUE;
- rc = NULL;
- goto cleanups;
+ return NULL;
}
ctx->allocbytes = allocbytes;
@@ -89,42 +86,35 @@ parser_init_guts(u64 addr, u32 bytes, BOOL is_local,
ERRDRV("%s - bad local address (0x%-16.16Lx for %lu)",
__func__,
(unsigned long long)addr, (ulong)bytes);
- rc = NULL;
- goto cleanups;
+ goto err_ctx;
}
p = __va((ulong)(addr));
memcpy(ctx->data, p, bytes);
} else {
rgn = visor_memregion_create(addr, bytes);
- if (!rgn) {
- rc = NULL;
- goto cleanups;
- }
- if (visor_memregion_read(rgn, 0, ctx->data, bytes) < 0) {
- rc = NULL;
- goto cleanups;
- }
+ if (!rgn)
+ goto err_ctx;
+
+ if (visor_memregion_read(rgn, 0, ctx->data, bytes) < 0)
+ goto err_rgn;
}
if (!has_standard_payload_header) {
ctx->byte_stream = TRUE;
- rc = ctx;
- goto cleanups;
+ goto out_rgn;
}
phdr = (struct spar_controlvm_parameters_header *)(ctx->data);
if (phdr->total_length != bytes) {
ERRDRV("%s - bad total length %lu (should be %lu)",
__func__,
(ulong)(phdr->total_length), (ulong)(bytes));
- rc = NULL;
- goto cleanups;
+ goto err_rgn;
}
if (phdr->total_length < phdr->header_length) {
ERRDRV("%s - total length < header length (%lu < %lu)",
__func__,
(ulong)(phdr->total_length),
(ulong)(phdr->header_length));
- rc = NULL;
- goto cleanups;
+ goto err_rgn;
}
if (phdr->header_length <
sizeof(struct spar_controlvm_parameters_header)) {
@@ -133,25 +123,23 @@ parser_init_guts(u64 addr, u32 bytes, BOOL is_local,
(ulong)(phdr->header_length),
(ulong)(sizeof(
struct spar_controlvm_parameters_header)));
- rc = NULL;
- goto cleanups;
+ goto err_rgn;
}
- rc = ctx;
-cleanups:
- if (rgn) {
+out_rgn:
+ if (rgn)
visor_memregion_destroy(rgn);
- rgn = NULL;
- }
- if (rc) {
- controlvm_payload_bytes_buffered += ctx->param_bytes;
- } else {
- if (ctx) {
- parser_done(ctx);
- ctx = NULL;
- }
- }
- return rc;
+ controlvm_payload_bytes_buffered += ctx->param_bytes;
+
+ return ctx;
+
+err_rgn:
+ if (rgn)
+ visor_memregion_destroy(rgn);
+err_ctx:
+ kfree(ctx);
+
+ return NULL;
}
struct parser_context_tag *
--
1.9.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 1/1] staging: unisys: parser.c bug
2014-11-17 19:46 Jeffrey Brown
2014-11-17 20:00 ` Greg KH
@ 2014-11-17 20:12 ` Dan Carpenter
1 sibling, 0 replies; 4+ messages in thread
From: Dan Carpenter @ 2014-11-17 20:12 UTC (permalink / raw)
To: Jeffrey Brown; +Cc: gregkh, driverdev-devel, sparmaintainer
Looks nice. Thanks, a lot!
regards,
dan carpenter
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 1/1] staging: unisys: parser.c bug
2014-11-17 19:46 Jeffrey Brown
@ 2014-11-17 20:00 ` Greg KH
2014-11-17 20:12 ` Dan Carpenter
1 sibling, 0 replies; 4+ messages in thread
From: Greg KH @ 2014-11-17 20:00 UTC (permalink / raw)
To: Jeffrey Brown; +Cc: driverdev-devel, sparmaintainer
On Mon, Nov 17, 2014 at 02:46:35PM -0500, Jeffrey Brown wrote:
> From: Jeff <Jeffrey.Brown@unisys.com>
This doesn't match your signed-off-by: line, nor your From: line of your
email above. Why even have this line when it isn't needed, and wrong?
greg k-h
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH 1/1] staging: unisys: parser.c bug
@ 2014-11-17 19:46 Jeffrey Brown
2014-11-17 20:00 ` Greg KH
2014-11-17 20:12 ` Dan Carpenter
0 siblings, 2 replies; 4+ messages in thread
From: Jeffrey Brown @ 2014-11-17 19:46 UTC (permalink / raw)
To: gregkh; +Cc: jkc, driverdev-devel, sparmaintainer, Jeff
From: Jeff <Jeffrey.Brown@unisys.com>
Replaced cleanups: with err_rgn, err_ctx, and out_rgn in the file.
This is to have proper error and success handling. I also removed
the variable rc because like cleanups, rc is redundant in this file
Signed-off-by: Jeffrey Brown <Jeffrey.Brown@unisys.com>
---
drivers/staging/unisys/visorchipset/parser.c | 62 +++++++++++-----------------
1 file changed, 25 insertions(+), 37 deletions(-)
diff --git a/drivers/staging/unisys/visorchipset/parser.c b/drivers/staging/unisys/visorchipset/parser.c
index 5f6a7b2..e4e230f 100644
--- a/drivers/staging/unisys/visorchipset/parser.c
+++ b/drivers/staging/unisys/visorchipset/parser.c
@@ -45,7 +45,6 @@ parser_init_guts(u64 addr, u32 bytes, BOOL is_local,
BOOL has_standard_payload_header, BOOL *try_again)
{
int allocbytes = sizeof(struct parser_context_tag) + bytes;
- struct parser_context_tag *rc = NULL;
struct parser_context_tag *ctx = NULL;
struct memregion *rgn = NULL;
struct spar_controlvm_parameters_header *phdr = NULL;
@@ -64,8 +63,7 @@ parser_init_guts(u64 addr, u32 bytes, BOOL is_local,
MAX_CONTROLVM_PAYLOAD_BYTES);
if (try_again)
*try_again = TRUE;
- rc = NULL;
- goto cleanups;
+ return NULL;
}
ctx = kzalloc(allocbytes, GFP_KERNEL|__GFP_NORETRY);
if (ctx == NULL) {
@@ -73,8 +71,7 @@ parser_init_guts(u64 addr, u32 bytes, BOOL is_local,
__func__, __FILE__, __LINE__, allocbytes);
if (try_again)
*try_again = TRUE;
- rc = NULL;
- goto cleanups;
+ return NULL;
}
ctx->allocbytes = allocbytes;
@@ -89,42 +86,35 @@ parser_init_guts(u64 addr, u32 bytes, BOOL is_local,
ERRDRV("%s - bad local address (0x%-16.16Lx for %lu)",
__func__,
(unsigned long long)addr, (ulong)bytes);
- rc = NULL;
- goto cleanups;
+ goto err_ctx;
}
p = __va((ulong)(addr));
memcpy(ctx->data, p, bytes);
} else {
rgn = visor_memregion_create(addr, bytes);
- if (!rgn) {
- rc = NULL;
- goto cleanups;
- }
- if (visor_memregion_read(rgn, 0, ctx->data, bytes) < 0) {
- rc = NULL;
- goto cleanups;
- }
+ if (!rgn)
+ goto err_ctx;
+
+ if (visor_memregion_read(rgn, 0, ctx->data, bytes) < 0)
+ goto err_rgn;
}
if (!has_standard_payload_header) {
ctx->byte_stream = TRUE;
- rc = ctx;
- goto cleanups;
+ goto out_rgn;
}
phdr = (struct spar_controlvm_parameters_header *)(ctx->data);
if (phdr->total_length != bytes) {
ERRDRV("%s - bad total length %lu (should be %lu)",
__func__,
(ulong)(phdr->total_length), (ulong)(bytes));
- rc = NULL;
- goto cleanups;
+ goto err_rgn;
}
if (phdr->total_length < phdr->header_length) {
ERRDRV("%s - total length < header length (%lu < %lu)",
__func__,
(ulong)(phdr->total_length),
(ulong)(phdr->header_length));
- rc = NULL;
- goto cleanups;
+ goto err_rgn;
}
if (phdr->header_length <
sizeof(struct spar_controlvm_parameters_header)) {
@@ -133,25 +123,23 @@ parser_init_guts(u64 addr, u32 bytes, BOOL is_local,
(ulong)(phdr->header_length),
(ulong)(sizeof(
struct spar_controlvm_parameters_header)));
- rc = NULL;
- goto cleanups;
+ goto err_rgn;
}
- rc = ctx;
-cleanups:
- if (rgn) {
+out_rgn:
+ if (rgn)
visor_memregion_destroy(rgn);
- rgn = NULL;
- }
- if (rc) {
- controlvm_payload_bytes_buffered += ctx->param_bytes;
- } else {
- if (ctx) {
- parser_done(ctx);
- ctx = NULL;
- }
- }
- return rc;
+ controlvm_payload_bytes_buffered += ctx->param_bytes;
+
+ return ctx;
+
+err_rgn:
+ if (rgn)
+ visor_memregion_destroy(rgn);
+err_ctx:
+ kfree(ctx);
+
+ return NULL;
}
struct parser_context_tag *
--
1.9.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-11-17 20:12 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-11-17 20:06 [PATCH 1/1] staging: unisys: parser.c bug Jeffrey Brown
-- strict thread matches above, loose matches on Subject: below --
2014-11-17 19:46 Jeffrey Brown
2014-11-17 20:00 ` Greg KH
2014-11-17 20:12 ` Dan Carpenter
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.