From ffc1f41ace8ee03d898e6932ecd03c8c4cf3acfe Mon Sep 17 00:00:00 2001
From: Jeremy Kerr <jeremy.kerr@canonical.com>
Date: Mon, 11 Jun 2012 16:33:18 +0800
Subject: [PATCH] Fix warnings from added -W flags
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Fix a few warnings:

 idc.c: In function ‘IDC_get’:
 idc.c:248:12: warning: ‘idclen’ may be used uninitialised in this function [-Wuninitialized]

 image.c: In function ‘image_load’:
 image.c:37:15: warning: unused variable ‘bytes_read’ [-Wunused-variable]

Plus, a bunch of strict-aliasing warnings:

 image.c:101:2: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]
 [ similar warnings trimmed ]

when compiling image.c. Since struct external_PEI_DOS_hdr uses char[]
types for all members, we need to use accessors here.

Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com>
---
 autogen.sh |  2 +-
 idc.c      |  4 ++++
 image.c    | 44 +++++++++++++++++++++++++++++++++++---------
 3 files changed, 40 insertions(+), 10 deletions(-)

diff --git a/autogen.sh b/autogen.sh
index 22e2759..8083fa3 100755
--- a/autogen.sh
+++ b/autogen.sh
@@ -1,6 +1,6 @@
 #!/bin/bash
 
-ccan_modules="talloc read_write_all"
+ccan_modules="talloc read_write_all build_assert"
 
 # Add ccan upstream sources
 if [ ! -e lib/ccan.git/Makefile ]
diff --git a/idc.c b/idc.c
index 289f9c2..c9b69d5 100644
--- a/idc.c
+++ b/idc.c
@@ -243,6 +243,10 @@ struct idc *IDC_get(PKCS7 *p7, BIO *bio)
 			idclen = (idcbuf[2] << 8) +
 				 idcbuf[3];
 			idcbuf += 4;
+		} else {
+			fprintf(stderr, "Invalid ASN.1 data in "
+					"IndirectDataContext?\n");
+			return NULL;
 		}
 
 		BIO_write(bio, idcbuf, idclen);
diff --git a/image.c b/image.c
index 591926e..a873a85 100644
--- a/image.c
+++ b/image.c
@@ -26,6 +26,7 @@
 
 #include <ccan/talloc/talloc.h>
 #include <ccan/read_write_all/read_write_all.h>
+#include <ccan/build_assert/build_assert.h>
 #include <openssl/sha.h>
 
 #include "image.h"
@@ -34,7 +35,6 @@
 
 struct image *image_load(const char *filename)
 {
-	unsigned int bytes_read;
 	struct stat statbuf;
 	struct image *image;
 	int rc;
@@ -78,6 +78,32 @@ err:
 	return NULL;
 }
 
+/**
+ * The PE/COFF headers export struct fields as arrays of chars. So, define
+ * a couple of accessor functions that allow fields to be deferenced as their
+ * native types, to allow strict aliasing. This also allows for endian-
+ * neutral behaviour.
+ */
+static uint32_t __pehdr_u32(char field[])
+{
+	uint8_t *ufield = (uint8_t *)field;
+	return (ufield[3] << 24) +
+		(ufield[2] << 16) +
+		(ufield[1] << 8) +
+		ufield[0];
+}
+
+static uint16_t __pehdr_u16(char field[])
+{
+	uint8_t *ufield = (uint8_t *)field;
+	return (ufield[1] << 8) +
+		ufield[0];
+}
+
+/* wrappers to ensure type correctness */
+#define pehdr_u32(f) __pehdr_u32(f + BUILD_ASSERT_OR_ZERO(sizeof(f) == 4))
+#define pehdr_u16(f) __pehdr_u16(f + BUILD_ASSERT_OR_ZERO(sizeof(f) == 2))
+
 int image_pecoff_parse(struct image *image)
 {
 	char nt_sig[] = {'P', 'E', 0, 0};
@@ -98,7 +124,7 @@ int image_pecoff_parse(struct image *image)
 		return -1;
 	}
 
-	addr = *(uint32_t *)image->doshdr->e_lfanew;
+	addr = pehdr_u32(image->doshdr->e_lfanew);
 	if (addr >= image->size) {
 		fprintf(stderr, "pehdr is beyond end of file [0x%08x]\n",
 				addr);
@@ -116,12 +142,12 @@ int image_pecoff_parse(struct image *image)
 		return -1;
 	}
 
-	if (*(uint16_t *)image->pehdr->f_magic != AMD64MAGIC) {
+	if (pehdr_u16(image->pehdr->f_magic) != AMD64MAGIC) {
 		fprintf(stderr, "Invalid PE header magic for x86_64\n");
 		return -1;
 	}
 
-	if (*(uint16_t *)image->pehdr->f_opthdr != sizeof(*image->aouthdr)) {
+	if (pehdr_u16(image->pehdr->f_opthdr) != sizeof(*image->aouthdr)) {
 		fprintf(stderr, "Invalid a.out header size\n");
 		return -1;
 	}
@@ -151,7 +177,7 @@ int image_pecoff_parse(struct image *image)
 	else
 		image->cert_table = NULL;
 
-	image->sections = *(uint16_t *)image->pehdr->f_nscns;
+	image->sections = pehdr_u16(image->pehdr->f_nscns);
 	image->scnhdr = (void *)(image->aouthdr+1);
 
 	return 0;
@@ -187,7 +213,7 @@ int image_find_regions(struct image *image)
 	size_t bytes;
 
 	gap_warn = 0;
-	align = *(uint32_t *)image->aouthdr->FileAlignment;
+	align = pehdr_u32(image->aouthdr->FileAlignment);
 
 	/* now we know where the checksum and cert table data is, we can
 	 * construct regions that need to be signed */
@@ -222,7 +248,7 @@ int image_find_regions(struct image *image)
 				(void *)image->data_dir_sigtable
 					+ sizeof(struct data_dir_entry),
 				image->buf +
-				*(uint32_t *)image->aouthdr->SizeOfHeaders);
+				pehdr_u32(image->aouthdr->SizeOfHeaders));
 	regions[2].name = "datadir[CERT]->headers";
 	bytes += regions[2].size;
 
@@ -230,8 +256,8 @@ int image_find_regions(struct image *image)
 	for (i = 0; i < image->sections; i++) {
 		uint32_t file_offset, file_size;
 
-		file_offset = *(uint32_t *)image->scnhdr[i].s_scnptr;
-		file_size = *(uint32_t *)image->scnhdr[i].s_size;
+		file_offset = pehdr_u32(image->scnhdr[i].s_scnptr);
+		file_size = pehdr_u32(image->scnhdr[i].s_size);
 
 		if (!file_size)
 			continue;