Remove part.h, more unaligned accesses, more named constants, proper exit status.

6 files changed, 110 insertions, 108 deletions

diff --git a/Makefile b/Makefile
index fe356f0..e982750 100644
--- a/Makefile
+++ b/Makefile
@@ -18,7 +18,7 @@ musl-static: static
 
 mkgpt: mkgpt.o crc32.o guid.o part_ids.o
 
-mkgpt.o: mkgpt.c guid.h part.h crc32.h
+mkgpt.o: mkgpt.c guid.h crc32.h
 guid.o: guid.c guid.h
 crc32.o: crc32.c crc32.h
 part_ids.o: part_ids.c part_ids.h guid.h
diff --git a/README.md b/README.md
index 32bbf0d..7b0ea19 100644
--- a/README.md
+++ b/README.md
@@ -88,3 +88,7 @@ Want to scare yourself?
 
 - https://uefi.org/sites/default/files/resources/UEFI_Spec_2_8_final.pdf
   (enjoy 2,500 pages of dread)
+
+Similar tool with JSON input?
+
+- https://gitlab.com/bztsrc/bootboot/tree/master/mkbootimg
diff --git a/guid.c b/guid.c
index ed9e6d2..a8a9b65 100644
--- a/guid.c
+++ b/guid.c
@@ -22,6 +22,7 @@
  */
 
 #include "guid.h"
+#include "unaligned.h"
 
 #include <stdio.h>
 #include <stdlib.h>
@@ -75,10 +76,9 @@ guid_to_bytestring(uint8_t *bytes, const GUID *guid)
 		return -1;
 	}
 
-	/* TODO potential alignment problem? */
-	*(uint32_t *)&bytes[0] = guid->data1;
-	*(uint16_t *)&bytes[4] = guid->data2;
-	*(uint16_t *)&bytes[6] = guid->data3;
+	set_u32(bytes + 0, guid->data1);
+	set_u16(bytes + 4, guid->data2);
+	set_u16(bytes + 6, guid->data3);
 	for (int i = 0; i < 8; i++) {
 		bytes[8 + i] = guid->data4[i];
 	}
diff --git a/mkgpt.c b/mkgpt.c
index 3f8ad36..ae71032 100644
--- a/mkgpt.c
+++ b/mkgpt.c
@@ -23,7 +23,6 @@
 
 #include "crc32.h"
 #include "guid.h"
-#include "part.h"
 #include "part_ids.h"
 #include "unaligned.h"
 
@@ -35,10 +34,36 @@
 #include <stdlib.h>
 #include <string.h>
 
+struct partition {
+	GUID type;
+	GUID uuid;
+	uint64_t attrs;
+	long src_length;
+	FILE *src;
+	struct partition *next; /* TODO why build a list? */
+	int id;
+	int sect_start;
+	int sect_length;
+	char name[52];
+};
+
 #define MAX_PART_NAME (36U)
 #define MIN_SECTOR_SIZE (512U)
 #define MAX_SECTOR_SIZE (4096U)
 
+/*
+ * UEFI says 128 is the "minimum size" but since we're generating the image we
+ * get to pick; and we're fine with 128 for now; anything else would probably
+ * also mess with other GPT tools?
+ */
+#define PART_ENTRY_SIZE (128U)
+
+/*
+ * TODO Everything else says 92 instead, and that's also what gdisk does when
+ * it creates a GPT. It's a mystery why the code here uses 96 instead.
+ */
+#define GPT_HEADER_SIZE (96U)
+
 static void
 dump_help(char *fname);
 static int
@@ -68,35 +93,37 @@ static int secondary_headers_sect;
 static int secondary_gpt_sect;
 
 int
-main(int argc, char **argv)
+main(int argc, char *argv[])
 {
 	random_guid(&disk_guid);
 
-	if (parse_opts(argc, argv) != 0)
-		return -1;
+	if (parse_opts(argc, argv) != 0) {
+		exit(EXIT_FAILURE);
+	}
 
 	if (output == NULL) {
-		fprintf(stderr, "no output file specifed\n");
+		fprintf(stderr, "no output file specified\n");
 		dump_help(argv[0]);
-		return -1;
+		exit(EXIT_FAILURE);
 	}
 	if (first_part == NULL) {
 		fprintf(stderr, "no partitions specified\n");
 		dump_help(argv[0]);
-		return -1;
+		exit(EXIT_FAILURE);
 	}
 
-	if (check_parts() != 0)
-		return -1;
+	if (check_parts() != 0) {
+		exit(EXIT_FAILURE);
+	}
 
 	write_output();
 	fclose(output);
 
-	return 0;
+	exit(EXIT_SUCCESS);
 }
 
 static int
-parse_opts(int argc, char **argv)
+parse_opts(int argc, char *argv[])
 {
 	int i = 1;
 	int cur_part_id = 0;
@@ -264,7 +291,7 @@ parse_opts(int argc, char **argv)
 			 */
 			if (strlen(argv[i]) > MAX_PART_NAME) {
 				fprintf(stderr,
-					"partition name too long (max %i)\n",
+					"partition name too long (max %u)\n",
 					MAX_PART_NAME);
 				return -1;
 			}
@@ -377,16 +404,18 @@ check_parts()
 	/* Determine the sectors needed for MBR, GPT header and partition
 	 * entries */
 	cur_sect = 2; /* MBR + GPT header */
-	header_length = part_count * 128;
+	header_length = part_count * PART_ENTRY_SIZE;
 	header_sectors = header_length / sect_size;
-	if (header_length % sect_size)
+	if (header_length % sect_size) {
 		header_sectors++;
+	}
 
 	/* The GPT entry array must be a minimum of 16,384 bytes (reports
 	 * wikipedia and testdisk, but not the UEFI spec)
 	 */
-	if (header_sectors < (int)(16384 / sect_size))
+	if (header_sectors < (int)(16384 / sect_size)) {
 		header_sectors = (int)(16384 / sect_size);
+	}
 
 	cur_sect += header_sectors;
 	first_usable_sector = cur_sect;
@@ -405,12 +434,14 @@ check_parts()
 			return -1;
 		}
 
-		if (guid_is_zero(&cur_part->uuid))
+		/* TODO is this appropriate? check the spec! */
+		if (guid_is_zero(&cur_part->uuid)) {
 			random_guid(&cur_part->uuid);
+		}
 
-		if (cur_part->sect_start == 0)
+		if (cur_part->sect_start == 0) {
 			cur_part->sect_start = cur_sect;
-		else if (cur_part->sect_start < cur_sect) {
+		} else if (cur_part->sect_start < cur_sect) {
 			fprintf(stderr,
 				"unable to start partition %i at sector %i "
 				"(would conflict with other data)\n",
@@ -424,8 +455,9 @@ check_parts()
 
 		if (cur_part->sect_length == 0) {
 			cur_part->sect_length = cur_part_file_len / sect_size;
-			if (cur_part_file_len % sect_size)
+			if (cur_part_file_len % sect_size) {
 				cur_part->sect_length++;
+			}
 		}
 		cur_sect = cur_part->sect_start + cur_part->sect_length;
 
@@ -436,10 +468,11 @@ check_parts()
 	needed_file_length = cur_sect + 1 + header_sectors;
 
 	if (image_sects == 0) {
-		if (needed_file_length > min_image_sects)
+		if (needed_file_length > min_image_sects) {
 			image_sects = needed_file_length;
-		else
+		} else {
 			image_sects = min_image_sects;
+		}
 	} else if (image_sects < needed_file_length) {
 		fprintf(stderr,
 			"requested image size (%zu) is too small to hold the "
@@ -508,20 +541,20 @@ write_output(void)
 	uint8_t gpt2[MAX_SECTOR_SIZE] = {0};
 	assert(sect_size <= sizeof(gpt2));
 
-	*(uint64_t *)&gpt[0] = 0x5452415020494645ULL; /* Signature */
-	*(uint32_t *)&gpt[8] = 0x00010000UL; /* Revision */
-	*(uint32_t *)&gpt[12] = 96; /* HeaderSize */
-	*(uint32_t *)&gpt[16] = 0; /* HeaderCRC32 */
-	*(uint32_t *)&gpt[20] = 0; /* Reserved */
-	*(uint64_t *)&gpt[24] = 0x1; /* MyLBA */
-	*(uint64_t *)&gpt[32] = secondary_gpt_sect; /* AlternateLBA */
-	*(uint64_t *)&gpt[40] = first_usable_sector; /* FirstUsableLBA */
-	*(uint64_t *)&gpt[48] = secondary_headers_sect - 1; /* LastUsableLBA */
-	guid_to_bytestring(&gpt[56], &disk_guid); /* DiskGUID */
-	*(uint64_t *)&gpt[72] = 0x2; /* PartitionEntryLBA */
-	*(uint32_t *)&gpt[80] = part_count; /* NumberOfPartitionEntries */
-	*(uint32_t *)&gpt[84] = 128; /* SizeOfPartitionEntry */
-	*(uint32_t *)&gpt[88] = 0; /* PartitionEntryArrayCRC32 */
+	set_u64(gpt + 0, 0x5452415020494645ULL); /* Signature */
+	set_u32(gpt + 8, 0x00010000UL); /* Revision */
+	set_u32(gpt + 12, GPT_HEADER_SIZE); /* HeaderSize */
+	set_u32(gpt + 16, 0); /* HeaderCRC32 */
+	set_u32(gpt + 20, 0); /* Reserved */
+	set_u64(gpt + 24, 0x1); /* MyLBA */
+	set_u64(gpt + 32, secondary_gpt_sect); /* AlternateLBA */
+	set_u64(gpt + 40, first_usable_sector); /* FirstUsableLBA */
+	set_u64(gpt + 48, secondary_headers_sect - 1); /* LastUsableLBA */
+	guid_to_bytestring(gpt + 56, &disk_guid); /* DiskGUID */
+	set_u64(gpt + 72, 0x2); /* PartitionEntryLBA */
+	set_u32(gpt + 80, part_count); /* NumberOfPartitionEntries */
+	set_u32(gpt + 84, PART_ENTRY_SIZE); /* SizeOfPartitionEntry */
+	set_u32(gpt + 88, 0); /* PartitionEntryArrayCRC32 */
 
 	/* Define GPT partition entries */
 	parts = calloc(header_sectors, sect_size);
@@ -534,29 +567,32 @@ write_output(void)
 	while (cur_part) {
 		int char_id;
 
-		guid_to_bytestring(&parts[i * 128],
+		guid_to_bytestring(parts + i * PART_ENTRY_SIZE + 0,
 			&cur_part->type); /* PartitionTypeGUID */
-		guid_to_bytestring(&parts[i * 128 + 16],
+		guid_to_bytestring(parts + i * PART_ENTRY_SIZE + 16,
 			&cur_part->uuid); /* UniquePartitionGUID */
-		*(uint64_t *)&parts[i * 128 + 32] =
-			cur_part->sect_start; /* StartingLBA */
-		*(uint64_t *)&parts[i * 128 + 40] = cur_part->sect_start +
-						    cur_part->sect_length -
-						    1; /* EndingLBA */
-		*(uint64_t *)&parts[i * 128 + 48] =
-			cur_part->attrs; /* Attributes */
+		set_u64(parts + i * PART_ENTRY_SIZE + 32,
+			cur_part->sect_start); /* StartingLBA */
+		set_u64(parts + i * PART_ENTRY_SIZE + 40,
+			cur_part->sect_start + cur_part->sect_length -
+				1); /* EndingLBA */
+		set_u64(parts + i * PART_ENTRY_SIZE + 48,
+			cur_part->attrs); /* Attributes */
 
 		/*
 		 * TODO settle missing UTF-16LE conversion issue somehow,
 		 * possibly by simply limiting the tool to ASCII here?
 		 * TODO used to be "&& char_id < 35" but MAX_PART_NAME is 36
 		 * now so which is correct? do we need a "double zero" at the
-		 * end or not? what does the spec say?
+		 * end or not? what does the spec say? sfdisk works with 36
+		 * chars and if we try 37 it just ignores the extra one, so
+		 * there's some indication that no "double zero" is needed and
+		 * that 36 is indeed the correct limit
 		 */
 		int len = min(strlen(cur_part->name), MAX_PART_NAME);
 		for (char_id = 0; char_id < len; char_id++) {
-			*(uint16_t *)&parts[i * 128 + 56 + char_id * 2] =
-				(uint16_t)cur_part->name[char_id];
+			set_u16(parts + i * PART_ENTRY_SIZE + 56 + char_id * 2,
+				cur_part->name[char_id]);
 		}
 
 		i++;
@@ -564,15 +600,23 @@ write_output(void)
 	}
 
 	/* Do CRC calculations on the partition table entries and GPT headers */
-	CalculateCrc32(parts, part_count * 128, (uint32_t *)&gpt[88]);
-	CalculateCrc32(gpt, 96, (uint32_t *)&gpt[16]);
-
-	memcpy(gpt2, gpt, 96);
-	*(uint32_t *)&gpt2[16] = 0; /* HeaderCRC32 */
-	*(uint64_t *)&gpt2[24] = secondary_gpt_sect; /* MyLBA */
-	*(uint64_t *)&gpt2[32] = 0x1; /* AlternateLBA */
-	*(uint64_t *)&gpt2[72] = secondary_headers_sect; /* PartitionEntryLBA */
-	CalculateCrc32(gpt2, 96, (uint32_t *)&gpt2[16]);
+	uint32_t parts_crc;
+	CalculateCrc32(parts, part_count * PART_ENTRY_SIZE, &parts_crc);
+	set_u32(gpt + 88, parts_crc);
+
+	uint32_t gpt_crc;
+	CalculateCrc32(gpt, GPT_HEADER_SIZE, &gpt_crc);
+	set_u32(gpt + 16, gpt_crc);
+
+	memcpy(gpt2, gpt, GPT_HEADER_SIZE);
+	set_u32(gpt2 + 16, 0); /* HeaderCRC32 */
+	set_u64(gpt2 + 24, secondary_gpt_sect); /* MyLBA */
+	set_u64(gpt2 + 32, 0x1); /* AlternateLBA */
+	set_u64(gpt2 + 72, secondary_headers_sect); /* PartitionEntryLBA */
+
+	uint32_t gpt2_crc;
+	CalculateCrc32(gpt2, GPT_HEADER_SIZE, &gpt2_crc);
+	set_u32(gpt2 + 16, gpt2_crc);
 
 	/* Write primary GPT and headers */
 	if (fwrite(gpt, 1, sect_size, output) != sect_size) {
diff --git a/part.h b/part.h
deleted file mode 100644
index a04ef02..0000000
--- a/part.h
+++ /dev/null
@@ -1,47 +0,0 @@
-#pragma once
-
-/* SPDX-License-Identifier: MIT */
-
-/* Copyright (C) 2014 by John Cronin
- *
- * Permission is hereby granted, free of charge, to any person obtaining a copy
- * of this software and associated documentation files (the "Software"), to deal
- * in the Software without restriction, including without limitation the rights
- * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
- * copies of the Software, and to permit persons to whom the Software is
- * furnished to do so, subject to the following conditions:
-
- * The above copyright notice and this permission notice shall be included in
- * all copies or substantial portions of the Software.
-
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
- * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
- * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
- * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
- * THE SOFTWARE.
- */
-
-#ifndef PART_H
-#define PART_H
-
-#include "guid.h"
-
-#include <stdint.h>
-#include <stdio.h>
-
-struct partition {
-	GUID type;
-	GUID uuid;
-	uint64_t attrs;
-	long src_length;
-	FILE *src;
-	struct partition *next; /* TODO why build a list? */
-	int id;
-	int sect_start;
-	int sect_length;
-	char name[52];
-};
-
-#endif
diff --git a/test-mkgpt.sh b/test-mkgpt.sh
index d952705..2ea491a 100755
--- a/test-mkgpt.sh
+++ b/test-mkgpt.sh
@@ -25,6 +25,7 @@ done
 
 fdisk -l ${tmpdir}/bla.img
 sfdisk --verify ${tmpdir}/bla.img
+sfdisk --dump ${tmpdir}/bla.img
 head -c 512 ${tmpdir}/bla.img | xxd -s 446
 
 checksum=$(md5sum ${tmpdir}/bla.img | cut -c1-32)