Stefan Reinauer has uploaded this change for review. ( https://review.coreboot.org/c/em100/+/47845 )
Change subject: Cleanly detach em100 in case of error ......................................................................
Cleanly detach em100 in case of error
Signed-off-by: Stefan Reinauer stefan.reinauer@coreboot.org Change-Id: Ibaeefaa9da257dfb3a89275146b40fd0713fa033 --- M em100.c 1 file changed, 20 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/em100 refs/changes/45/47845/1
diff --git a/em100.c b/em100.c index 1af0f03..178151a 100644 --- a/em100.c +++ b/em100.c @@ -934,8 +934,10 @@ }
const chipdesc *chip = setup_chips(desiredchip); - if (desiredchip && !chip) + if (desiredchip && !chip) { + em100_detach(em100); return 1; + }
/* Set up signal handler. This is used for two reasons: * 1) to create a way to cleanly exit trace mode. @@ -1026,6 +1028,7 @@ if (desiredchip) { if (!set_chip_type(em100, chip)) { printf("Failed configuring chip type.\n"); + em100_detach(em100); return 0; } printf("Chip set to %s %s.\n", chip->vendor, chip->name); @@ -1040,6 +1043,7 @@
if (address_mode) { if (set_address_mode(em100, address_mode) < 0) { + em100_detach(em100); return 1; } } @@ -1047,6 +1051,7 @@ if (voltage) { if (!set_fpga_voltage_from_str(em100, voltage)) { printf("Failed configuring FPGA voltage.\n"); + em100_detach(em100); return 1; } } @@ -1054,6 +1059,7 @@ if (holdpin) { if (!set_hold_pin_state_from_str(em100, holdpin)) { printf("Failed configuring hold pin state.\n"); + em100_detach(em100); return 0; } } @@ -1076,12 +1082,14 @@ void *data = malloc(maxlen); if (data == NULL) { printf("FATAL: couldn't allocate memory\n"); + em100_detach(em100); return 1; } FILE *fdata = fopen(read_filename, "wb"); if (!fdata) { perror("Could not open download file"); free(data); + em100_detach(em100); return 1; }
@@ -1093,6 +1101,7 @@
if (length != 1) { printf("FATAL: failed to write"); + em100_detach(em100); return 1; } } @@ -1105,12 +1114,14 @@
if (data == NULL) { printf("FATAL: couldn't allocate memory\n"); + em100_detach(em100); return 1; } FILE *fdata = fopen(filename, "rb"); if (!fdata) { perror("Could not open upload file"); free(data); + em100_detach(em100); return 1; }
@@ -1125,12 +1136,14 @@ if (length > maxlen) { printf("FATAL: length > maxlen\n"); free(data); + em100_detach(em100); return 1; }
if (length == 0) { printf("FATAL: No file to upload.\n"); free(data); + em100_detach(em100); return 1; }
@@ -1138,6 +1151,7 @@ { printf("FATAL: file size does not match to chip size.\n"); free(data); + em100_detach(em100); return 1; }
@@ -1149,6 +1163,7 @@ if (readback == NULL) { printf("FATAL: couldn't allocate memory(size: %x)\n", maxlen); free(data); + em100_detach(em100); return 1; } done = read_sdram(em100, readback, 0, maxlen); @@ -1168,6 +1183,7 @@ if (readback == NULL) { printf("FATAL: couldn't allocate memory\n"); free(data); + em100_detach(em100); return 1; } done = read_sdram(em100, readback, spi_start_address, length); @@ -1195,6 +1211,7 @@
if ((holdpin == NULL) && (!set_hold_pin_state(em100, 3))) { printf("Error: Failed to set EM100 to input\n"); + em100_detach(em100); return 1; }
@@ -1230,11 +1247,13 @@
if (!do_start && !do_stop) set_state(em100, 0); + if (trace) reset_spi_trace(em100);
if ((holdpin == NULL) && (!set_hold_pin_state(em100, 2))) { printf("Error: Failed to set EM100 to float\n"); + em100_detach(em100); return 1; } }
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/em100/+/47845 )
Change subject: Cleanly detach em100 in case of error ......................................................................
Patch Set 1: Code-Review+1
I wonder if making a helper function to avoid having to detach on every return point makes sense.
Stefan Reinauer has posted comments on this change. ( https://review.coreboot.org/c/em100/+/47845 )
Change subject: Cleanly detach em100 in case of error ......................................................................
Patch Set 1:
Yeah... I'm trying to figure out if atexit() can do the job. Same potentially for all the free() calls. But I think it still makes to submit this until then
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/em100/+/47845 )
Change subject: Cleanly detach em100 in case of error ......................................................................
Patch Set 1: Code-Review+2
Patch Set 1:
Yeah... I'm trying to figure out if atexit() can do the job. Same potentially for all the free() calls. But I think it still makes to submit this until then
Even just splitting main() into a helper subroutine would do the trick, I think.
Stefan Reinauer has submitted this change. ( https://review.coreboot.org/c/em100/+/47845 )
Change subject: Cleanly detach em100 in case of error ......................................................................
Cleanly detach em100 in case of error
Signed-off-by: Stefan Reinauer stefan.reinauer@coreboot.org Change-Id: Ibaeefaa9da257dfb3a89275146b40fd0713fa033 Reviewed-on: https://review.coreboot.org/c/em100/+/47845 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Angel Pons th3fanbus@gmail.com --- M em100.c 1 file changed, 20 insertions(+), 1 deletion(-)
Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved
diff --git a/em100.c b/em100.c index 6d35b76..c4b6c01 100644 --- a/em100.c +++ b/em100.c @@ -934,8 +934,10 @@ }
const chipdesc *chip = setup_chips(desiredchip); - if (desiredchip && !chip) + if (desiredchip && !chip) { + em100_detach(em100); return 1; + }
/* Set up signal handler. This is used for two reasons: * 1) to create a way to cleanly exit trace mode. @@ -1026,6 +1028,7 @@ if (desiredchip) { if (!set_chip_type(em100, chip)) { printf("Failed configuring chip type.\n"); + em100_detach(em100); return 0; } printf("Chip set to %s %s.\n", chip->vendor, chip->name); @@ -1040,6 +1043,7 @@
if (address_mode) { if (set_address_mode(em100, address_mode) < 0) { + em100_detach(em100); return 1; } } @@ -1047,6 +1051,7 @@ if (voltage) { if (!set_fpga_voltage_from_str(em100, voltage)) { printf("Failed configuring FPGA voltage.\n"); + em100_detach(em100); return 1; } } @@ -1054,6 +1059,7 @@ if (holdpin) { if (!set_hold_pin_state_from_str(em100, holdpin)) { printf("Failed configuring hold pin state.\n"); + em100_detach(em100); return 0; } } @@ -1076,12 +1082,14 @@ void *data = malloc(maxlen); if (data == NULL) { printf("FATAL: couldn't allocate memory\n"); + em100_detach(em100); return 1; } FILE *fdata = fopen(read_filename, "wb"); if (!fdata) { perror("Could not open download file"); free(data); + em100_detach(em100); return 1; }
@@ -1093,6 +1101,7 @@
if (length != 1) { printf("FATAL: failed to write"); + em100_detach(em100); return 1; } } @@ -1105,12 +1114,14 @@
if (data == NULL) { printf("FATAL: couldn't allocate memory\n"); + em100_detach(em100); return 1; } FILE *fdata = fopen(filename, "rb"); if (!fdata) { perror("Could not open upload file"); free(data); + em100_detach(em100); return 1; }
@@ -1125,12 +1136,14 @@ if (length > maxlen) { printf("FATAL: length > maxlen\n"); free(data); + em100_detach(em100); return 1; }
if (length == 0) { printf("FATAL: No file to upload.\n"); free(data); + em100_detach(em100); return 1; }
@@ -1138,6 +1151,7 @@ { printf("FATAL: file size does not match to chip size.\n"); free(data); + em100_detach(em100); return 1; }
@@ -1149,6 +1163,7 @@ if (readback == NULL) { printf("FATAL: couldn't allocate memory(size: %x)\n", maxlen); free(data); + em100_detach(em100); return 1; } done = read_sdram(em100, readback, 0, maxlen); @@ -1168,6 +1183,7 @@ if (readback == NULL) { printf("FATAL: couldn't allocate memory\n"); free(data); + em100_detach(em100); return 1; } done = read_sdram(em100, readback, spi_start_address, length); @@ -1195,6 +1211,7 @@
if ((holdpin == NULL) && (!set_hold_pin_state(em100, 3))) { printf("Error: Failed to set EM100 to input\n"); + em100_detach(em100); return 1; }
@@ -1230,11 +1247,13 @@
if (!do_start && !do_stop) set_state(em100, 0); + if (trace) reset_spi_trace(em100);
if ((holdpin == NULL) && (!set_hold_pin_state(em100, 2))) { printf("Error: Failed to set EM100 to float\n"); + em100_detach(em100); return 1; } }
-
Angel Pons (Code Review) -
Stefan Reinauer (Code Review)