Better, but it still needs more work.
On Fri, Nov 14, 2014 at 07:41:28AM -0500, Jeffrey Brown wrote:
> Replaced "cleanups" in the struct parser_init_guts with err_rgn,
> err_ctx, and out_rgn. The purpose is to remove redundant code and
> have proper error handling
>
> Signed-off-by: Jeffrey Brown <Jeffrey.Brown@xxxxxxxxxx>
> ---
> drivers/staging/unisys/visorchipset/parser.c | 43 +++++++++++--------------
> 1 files changed, 19 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/staging/unisys/visorchipset/parser.c b/drivers/staging/unisys/visorchipset/parser.c
> index 5f6a7b2..2897125 100644
> --- a/drivers/staging/unisys/visorchipset/parser.c
> +++ b/drivers/staging/unisys/visorchipset/parser.c
> @@ -64,8 +64,6 @@ 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) {
> @@ -74,7 +72,7 @@ parser_init_guts(u64 addr, u32 bytes, BOOL is_local,
> if (try_again)
> *try_again = TRUE;
> rc = NULL;
> - goto cleanups;
> + return NULL;
> }
>
> ctx->allocbytes = allocbytes;
> @@ -90,7 +88,6 @@ parser_init_guts(u64 addr, u32 bytes, BOOL is_local,
> __func__,
> (unsigned long long)addr, (ulong)bytes);
> rc = NULL;
> - goto cleanups;
goto err_ctx;
> }
> p = __va((ulong)(addr));
> memcpy(ctx->data, p, bytes);
> @@ -98,17 +95,17 @@ parser_init_guts(u64 addr, u32 bytes, BOOL is_local,
> rgn = visor_memregion_create(addr, bytes);
> if (!rgn) {
> rc = NULL;
> - goto cleanups;
> + goto err_ctx;
> }
> if (visor_memregion_read(rgn, 0, ctx->data, bytes) < 0) {
> rc = NULL;
> - goto cleanups;
> + goto err_ctx;
> }
> }
> if (!has_standard_payload_header) {
> ctx->byte_stream = TRUE;
> rc = ctx;
> - goto cleanups;
> + goto err_rgn;
This is not an error. goto out_ctx;
> }
> phdr = (struct spar_controlvm_parameters_header *)(ctx->data);
> if (phdr->total_length != bytes) {
> @@ -116,7 +113,7 @@ parser_init_guts(u64 addr, u32 bytes, BOOL is_local,
> __func__,
> (ulong)(phdr->total_length), (ulong)(bytes));
> rc = NULL;
> - goto cleanups;
> + goto out_rgn;
This is an error. goto err_rgn;
> }
> if (phdr->total_length < phdr->header_length) {
> ERRDRV("%s - total length < header length (%lu < %lu)",
> @@ -124,7 +121,7 @@ parser_init_guts(u64 addr, u32 bytes, BOOL is_local,
> (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)) {
> @@ -134,24 +131,22 @@ parser_init_guts(u64 addr, u32 bytes, BOOL is_local,
> (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;
Missing line:
controlvm_payload_bytes_buffered += ctx->param_bytes;
> +
> + return ctx;
> +
> +err_rgn:
> + if (rgn)
> + visor_memregion_destroy(rgn);
> +
> +err_ctx:
> + kfree(ctx);
> + return NULL;
regards,
dan carpenter
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
