CID1130005: Array compared against 0
The address of an array is never NULL, so the comparison will always evaluate
the same way.
In selfcheck: Array compared against NULL pointer
Since the array is defined unconditionally in C code the check does not really
make sense. It might make more sense to check whether there are entries in the
array, but that is not required on all platforms so far.
Signed-off-by: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Index: print.c
===================================================================
--- print.c (revision 1763)
+++ print.c (working copy)
@@ -1073,7 +1073,7 @@
B("ZOTAC", "ZBOX HD-ID11", OK, NULL, NULL),
#endif
- {0},
+ {NULL},
};
/* Please keep this list alphabetically ordered by vendor/board. */
@@ -1101,6 +1101,6 @@
B("Lenovo", "3000 V100 TF05Cxx", OK, "http://www5.pc.ibm.com/europe/products.nsf/products?openagent&brand=Lenovo3…", NULL),
#endif
- {0},
+ {NULL},
};
#endif
Index: flashrom.c
===================================================================
--- flashrom.c (revision 1763)
+++ flashrom.c (working copy)
@@ -1759,24 +1759,6 @@
if (selfcheck_eraseblocks(chip))
ret = 1;
-#if CONFIG_INTERNAL == 1
- if (chipset_enables == NULL) {
- msg_gerr("Chipset enables table does not exist!\n");
- ret = 1;
- }
- if (board_matches == NULL) {
- msg_gerr("Board enables table does not exist!\n");
- ret = 1;
- }
- if (boards_known == NULL) {
- msg_gerr("Known boards table does not exist!\n");
- ret = 1;
- }
- if (laptops_known == NULL) {
- msg_gerr("Known laptops table does not exist!\n");
- ret = 1;
- }
-#endif
return ret;
}
CID1129998/1129999: Unchecked return value from library
The function returns a value that indicates an error condition. If this is not
checked, the error condition may not be handled correctly.
In serialport_write_nonblock: Value returned from a library function is not
checked for errors before being used. This value may indicate an error
condition.
In serialport_config: Value returned from a library function is not checked for
errors before being used. This value may indicate an error condition.
Signed-off-by: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Index: serial.c
===================================================================
--- serial.c (revision 1763)
+++ serial.c (working copy)
@@ -181,7 +181,10 @@
msg_pdbg("Baud rate is %ld.\n", dcb.BaudRate);
#else
struct termios wanted, observed;
- fcntl(fd, F_SETFL, 0);
+ if (fcntl(fd, F_SETFL, 0) != 0) {
+ msg_perr_strerror("Could not set serial port mode: ");
+ return 1;
+ }
if (tcgetattr(fd, &observed) != 0) {
msg_perr_strerror("Could not fetch original serial port configuration: ");
return 1;
@@ -419,14 +422,21 @@
return -1;
}
if(!SetCommTimeouts(sp_fd, &newTimeout)) {
+ msg_perr_strerror("Could not set serial port timeout settings: ");
+ return -1;
+ }
#else
ssize_t rv;
const int flags = fcntl(sp_fd, F_GETFL);
+ if (flags == -1) {
+ msg_perr_strerror("Could not get serial port mode: ");
+ return -1;
+ }
if (fcntl(sp_fd, F_SETFL, flags | O_NONBLOCK) != 0) {
-#endif
- msg_perr_strerror("Could not set serial port timeout settings %s");
+ msg_perr_strerror("Could not set serial port mode to non-blocking: ");
return -1;
}
+#endif
int i;
int rd_bytes = 0;
@@ -458,12 +468,15 @@
/* restore original blocking behavior */
#ifdef _WIN32
if (!SetCommTimeouts(sp_fd, &oldTimeout)) {
+ msg_perr_strerror("Could not set serial port timeout settings: ");
+ ret = -1;
+ }
#else
if (fcntl(sp_fd, F_SETFL, flags) != 0) {
-#endif
- msg_perr_strerror("Could not restore serial port timeout settings: %s\n");
+ msg_perr_strerror("Could not set serial port mode to non-blocking: ");
ret = -1;
}
+#endif
return ret;
}
@@ -495,7 +508,14 @@
#else
ssize_t rv;
const int flags = fcntl(sp_fd, F_GETFL);
- fcntl(sp_fd, F_SETFL, flags | O_NONBLOCK);
+ if (flags == -1) {
+ msg_perr_strerror("Could not get serial port mode: ");
+ return -1;
+ }
+ if (fcntl(sp_fd, F_SETFL, flags | O_NONBLOCK) != 0) {
+ msg_perr_strerror("Could not set serial port mode to non-blocking: ");
+ return -1;
+ }
#endif
int i;
@@ -531,10 +551,13 @@
#ifdef _WIN32
if (!SetCommTimeouts(sp_fd, &oldTimeout)) {
msg_perr_strerror("Could not restore serial port timeout settings: ");
+ return -1;
+ }
#else
if (fcntl(sp_fd, F_SETFL, flags) != 0) {
-#endif
+ msg_perr_strerror("Could not reset serial port mode: ");
return -1;
}
+#endif
return ret;
}
CID1130010: Resource leak
The system resource will not be reclaimed and reused, reducing the future
availability of the resource.
In verify_range: Leak of memory or pointers to system resources
Signed-off-by: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Index: flashrom.c
===================================================================
--- flashrom.c (revision 1763)
+++ flashrom.c (working copy)
@@ -676,7 +676,8 @@
if (!flash->chip->read) {
msg_cerr("ERROR: flashrom has no read function for this flash chip.\n");
- return 1;
+ ret = -1;
+ goto out_free;
}
if (!readbuf) {
msg_gerr("Could not allocate memory!\n");
Am 21.07.2011 17:54 schrieb Stefan Tauner:
> previously the dummies were initialized to be empty (all ones), which makes writes skip
> erasing altogether. with this patch the default is to fill it with random bytes instead and the
> old behavior can be enforced with stating "empty=yes" on the command line.
>
> Signed-off-by: Stefan Tauner <stefan.tauner(a)student.tuwien.ac.at>
>
Does the following not work for you?
dd if=/dev/urandom bs=1k count=4k of=oldimage.bin
flashrom -p dummy:emulate=SST25VF032B,image=oldimage.bin -w newimage.bin
Regards,
Carl-Daniel
--
http://www.hailfinger.org/
On Fri, 30 Aug 2013 02:00:34 +0200
Stefan Tauner <stefan.tauner(a)student.tuwien.ac.at> wrote:
> This was mainly driven by getting rid of cli_classic_abort_usage() in cli_classic.c.
I forgot to mention something and give a rationale for it:
Thereby we also get rid of teasing the user to run flashrom again to
show usage information.
If we think the user should see the usage information we should show it
to him or at least a short syntax summary (which we pretty much lack
currently: the programmer parameter syntax is not really well
explained). (Small) GNU apps seem to favor printing a similar message
like we do. nmap, rsync give their whole usage information which is a
bit of a pain because it is so long (unlike flashrom's), ping and hd
are examples that just print a syntax summary (but they lack a complete
usage help). All of the above is stupid. :P Printing a wall of text
does not help and is even counterproductive if one can not scroll.
Printing how to print help is also a little bit silly if that help text
is not very long anyway. The right™ thing in the case of flashrom is
IMHO to print the complete usage help (after improving it to explain
the programmer parameter syntax better).
RFC.
>
> This makes the whole main function more consistent and fixes a myriad of
> (theoretical) memory leaks. This slightly changes output in most error cases.
> doit() stops calling programmer_shutdown() with this patch, beware.
--
Kind regards/Mit freundlichen Grüßen, Stefan Tauner
Attached is the output from "flashrom -V" as the output requested.
I backed up the running BIOS with AFUDOS and flashrom getting different
sum values under fedora 18.
$ sum *.ROM
19558 4096 SGIBIOS.ROM
$ sum *.rom
16753 4096 MX25L3205A.rom
I'm getting ready to flash over the SGI BIOS with the latest Supermicro
and I'm worried about using flashrom instead of AFUDOS.
Let me know if anyone is interested in helping my efforts.
Thanks,