Patchwork cbtable.c: Do not unnecessarily duplicate strings

login
register
about
Submitter Paul Menzel
Date 2014-10-25 12:31:27
Message ID <1414240287.4154.11.camel@users.sourceforge.net>
Download mbox | patch
Permalink /patch/4247/
State Accepted
Headers show

Comments

Paul Menzel - 2014-10-25 12:31:27
From: Paul Menzel <paulepanter@users.sourceforge.net>
Date: Tue, 21 Oct 2014 23:46:18 +0200

Commit 1577 (Refactor the -p internal:mainboard handling.) got rid of
the global variables `image_vendor` and `image_model`. Therefore
duplicating strings to store it is not needed anymore.

Found-by: Valgrind 3.10.0

	==25829== Command: /src/flashrom/flashrom -p internal -c W25Q32.V -w build/coreboot.rom
	[…]
	==25829== 7 bytes in 1 blocks are definitely lost in loss record 3 of 4
	==25829==    at 0x40291CC: malloc (vg_replace_malloc.c:296)
	==25829==    by 0x40FD517: strdup (strdup.c:42)
	==25829==    by 0x8055869: cb_check_image (cbtable.c:86)
	==25829==    by 0x806466B: doit (flashrom.c:1972)
	==25829==    by 0x804A111: main (cli_classic.c:545)
	==25829==
	==25829== 7 bytes in 1 blocks are definitely lost in loss record 4 of 4
	==25829==    at 0x40291CC: malloc (vg_replace_malloc.c:296)
	==25829==    by 0x40FD517: strdup (strdup.c:42)
	==25829==    by 0x8055873: cb_check_image (cbtable.c:87)
	==25829==    by 0x806466B: doit (flashrom.c:1972)
	==25829==    by 0x804A111: main (cli_classic.c:545)

Signed-off-by: Paul Menzel <paulepanter@users.sourceforge.net>
---
 cbtable.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)
Stefan Tauner - 2014-11-01 23:12:48
On Sat, 25 Oct 2014 14:31:27 +0200
Paul Menzel <paulepanter@users.sourceforge.net> wrote:

> From: Paul Menzel <paulepanter@users.sourceforge.net>
> Date: Tue, 21 Oct 2014 23:46:18 +0200
> 
> Commit 1577 (Refactor the -p internal:mainboard handling.) got rid of
> the global variables `image_vendor` and `image_model`. Therefore
> duplicating strings to store it is not needed anymore.
> 
> Found-by: Valgrind 3.10.0

Thanks Paul,
Acked-by: Stefan Tauner <stefan.tauner@alumni.tuwien.ac.at>
and committed in r1854.

Patch

diff --git a/cbtable.c b/cbtable.c
index c100bbb..1a74e46 100644
--- a/cbtable.c
+++ b/cbtable.c
@@ -40,8 +40,6 @@  static char *cb_vendor = NULL, *cb_model = NULL;
  */
 int cb_check_image(uint8_t *image, int size)
 {
-	const char *image_vendor = NULL;
-	const char *image_model = NULL;
 	unsigned int *walk;
 	unsigned int mb_part_offset, mb_vendor_offset;
 	char *mb_part, *mb_vendor;
@@ -83,22 +81,20 @@  int cb_check_image(uint8_t *image, int size)
 
 	msg_pdbg("coreboot last image size (not ROM size) is %d bytes.\n", *walk);
 
-	image_vendor = strdup(mb_vendor);
-	image_model = strdup(mb_part);
-	msg_pdbg("Manufacturer: %s\n", image_vendor);
-	msg_pdbg("Mainboard ID: %s\n", image_model);
+	msg_pdbg("Manufacturer: %s\n", mb_vendor);
+	msg_pdbg("Mainboard ID: %s\n", mb_part);
 
 	/* If these are not set, the coreboot table was not found. */
 	if (!cb_vendor || !cb_model)
 		return 0;
 
 	/* These comparisons are case insensitive to make things a little less user^Werror prone. */
-	if (!strcasecmp(image_vendor, cb_vendor) && !strcasecmp(image_model, cb_model)) {
+	if (!strcasecmp(mb_vendor, cb_vendor) && !strcasecmp(mb_part, cb_model)) {
 		msg_pdbg2("This coreboot image matches this mainboard.\n");
 	} else {
 		msg_perr("This coreboot image (%s:%s) does not appear to\n"
 			 "be correct for the detected mainboard (%s:%s).\n",
-			 image_vendor, image_model, cb_vendor, cb_model);
+			 mb_vendor, mb_part, cb_vendor, cb_model);
 		return -1;
 	}