From: krw <krw@openbsd.org>
Date: Mon, 25 Apr 2022 17:10:09 +0000 (+0000)
Subject: Push DISK_[read|write]sectors() down to disk.c local functions
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=adf35c805063bdae2645f1ef12aa003e59773899;p=openbsd

Push DISK_[read|write]sectors() down to disk.c local functions
and expose new DISK_[read|write]bytes() functions.

Eliminates many bytes <-> sector i/o dances and makes the code
much easier to understand.

Be a bit more careful to consistently write only bytes that
checksums are calculated over.

No intentional functional change.
---

diff --git a/sbin/fdisk/disk.c b/sbin/fdisk/disk.c
index c505a877c87..ddcd0c3af9d 100644
--- a/sbin/fdisk/disk.c
+++ b/sbin/fdisk/disk.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: disk.c,v 1.74 2021/10/10 15:34:21 krw Exp $	*/
+/*	$OpenBSD: disk.c,v 1.75 2022/04/25 17:10:09 krw Exp $	*/
 
 /*
  * Copyright (c) 1997 Tobias Weingartner
@@ -38,6 +38,9 @@
 struct disk		disk;
 struct disklabel	dl;
 
+char		*readsectors(const uint64_t, const uint32_t);
+int		 writesectors(const void *, const uint64_t, const uint32_t);
+
 void
 DISK_open(const char *name, const int oflags)
 {
@@ -114,7 +117,7 @@ DISK_printgeometry(const char *units)
  * The caller must free() the returned memory!
  */
 char *
-DISK_readsectors(const uint64_t sector, const uint32_t count)
+readsectors(const uint64_t sector, const uint32_t count)
 {
 	char			*secbuf;
 	ssize_t			 len;
@@ -156,8 +159,7 @@ DISK_readsectors(const uint64_t sector, const uint32_t count)
 }
 
 int
-DISK_writesectors(const char *buf, const uint64_t sector,
-    const uint32_t count)
+writesectors(const void *buf, const uint64_t sector, const uint32_t count)
 {
 	ssize_t			len;
 	off_t			off, where;
@@ -190,3 +192,41 @@ DISK_writesectors(const char *buf, const uint64_t sector,
 
 	return 0;
 }
+
+int
+DISK_readbytes(void *buf, const uint64_t sector, const size_t sz)
+{
+	char			*secbuf;
+	uint32_t		 count;
+
+	count = (sz + dl.d_secsize - 1) / dl.d_secsize;
+
+	secbuf = readsectors(sector, count);
+	if (secbuf == NULL)
+		return -1;
+
+	memcpy(buf, secbuf, sz);
+	free(secbuf);
+
+	return 0;
+}
+
+int
+DISK_writebytes(const void *buf, const uint64_t sector, const size_t sz)
+{
+	char 			*secbuf;
+	uint32_t		 count;
+	int			 rslt;
+
+	count = (sz + dl.d_secsize - 1) / dl.d_secsize;
+
+	secbuf = readsectors(sector, count);
+	if (secbuf == NULL)
+		return -1;
+
+	memcpy(secbuf, buf, sz);
+	rslt = writesectors(secbuf, sector, count);
+	free(secbuf);
+
+	return rslt;
+}
diff --git a/sbin/fdisk/disk.h b/sbin/fdisk/disk.h
index 84de2a33175..7daf790ae98 100644
--- a/sbin/fdisk/disk.h
+++ b/sbin/fdisk/disk.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: disk.h,v 1.32 2021/10/25 13:51:25 krw Exp $	*/
+/*	$OpenBSD: disk.h,v 1.33 2022/04/25 17:10:09 krw Exp $	*/
 
 /*
  * Copyright (c) 1997 Tobias Weingartner
@@ -31,9 +31,8 @@ struct disk {
 
 void		 DISK_open(const char *, const int);
 void		 DISK_printgeometry(const char *);
-char		*DISK_readsectors(const uint64_t, const uint32_t);
-int		 DISK_writesectors(const char *, const uint64_t,
-	const uint32_t);
+int		 DISK_readbytes(void *, const uint64_t, const size_t);
+int		 DISK_writebytes(const void *, const uint64_t, const size_t);
 
 extern struct disk		disk;
 extern struct disklabel		dl;
diff --git a/sbin/fdisk/gpt.c b/sbin/fdisk/gpt.c
index 608e2828204..ac693bc5494 100644
--- a/sbin/fdisk/gpt.c
+++ b/sbin/fdisk/gpt.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: gpt.c,v 1.74 2022/04/25 13:07:53 krw Exp $	*/
+/*	$OpenBSD: gpt.c,v 1.75 2022/04/25 17:10:09 krw Exp $	*/
 /*
  * Copyright (c) 2015 Markus Muller <mmu@grummel.net>
  * Copyright (c) 2015 Kenneth R Westerback <krw@openbsd.org>
@@ -142,16 +142,11 @@ int
 get_header(const uint64_t sector)
 {
 	struct gpt_header	 legh;
-	char			*secbuf;
 	uint64_t		 gpbytes, gpsectors, lba_end;
 
-	secbuf = DISK_readsectors(sector, 1);
-	if (secbuf == NULL)
+	if (DISK_readbytes(&legh, sector, sizeof(legh)))
 		return -1;
 
-	memcpy(&legh, secbuf, sizeof(struct gpt_header));
-	free(secbuf);
-
 	gh.gh_sig = letoh64(legh.gh_sig);
 	if (gh.gh_sig != GPTSIGNATURE) {
 		DPRINTF("gpt signature: expected 0x%llx, got 0x%llx\n",
@@ -267,24 +262,18 @@ get_header(const uint64_t sector)
 int
 get_partition_table(void)
 {
-	char			*secbuf;
-	uint64_t		 gpbytes, gpsectors;
+	uint64_t		 gpbytes;
 	uint32_t		 gh_part_csum;
 
 	DPRINTF("gpt partition table being read from LBA %llu\n",
 	    gh.gh_part_lba);
 
 	gpbytes = gh.gh_part_num * gh.gh_part_size;
-	gpsectors = (gpbytes + dl.d_secsize - 1) / dl.d_secsize;
 	memset(&gp, 0, sizeof(gp));
 
-	secbuf = DISK_readsectors(gh.gh_part_lba, gpsectors);
-	if (secbuf == NULL)
+	if (DISK_readbytes(&gp, gh.gh_part_lba, gpbytes))
 		return -1;
 
-	memcpy(&gp, secbuf, gpbytes);
-	free(secbuf);
-
 	gh_part_csum = gh.gh_part_csum;
 	gh.gh_part_csum = crc32((unsigned char *)&gp, gpbytes);
 	if (gh_part_csum != gh.gh_part_csum) {
@@ -632,44 +621,35 @@ GPT_init(const int how)
 void
 GPT_zap_headers(void)
 {
-	char			*secbuf;
-	uint64_t		 sig;
+	struct gpt_header	legh;
 
-	secbuf = DISK_readsectors(GPTSECTOR, 1);
-	if (secbuf == NULL)
+	if (DISK_readbytes(&legh, GPTSECTOR, sizeof(legh)))
 		return;
 
-	memcpy(&sig, secbuf, sizeof(sig));
-	if (letoh64(sig) == GPTSIGNATURE) {
-		memset(secbuf, 0, dl.d_secsize);
-		if (DISK_writesectors(secbuf, GPTSECTOR, 1))
+	if (letoh64(legh.gh_sig) == GPTSIGNATURE) {
+		memset(&legh, 0, sizeof(legh));
+		if (DISK_writebytes(&legh, GPTSECTOR, sizeof(legh)))
 			DPRINTF("Unable to zap GPT header @ sector %d",
 			    GPTSECTOR);
 	}
-	free(secbuf);
 
-	secbuf = DISK_readsectors(DL_GETDSIZE(&dl) - 1, 1);
-	if (secbuf == NULL)
+	if (DISK_readbytes(&legh, DL_GETDSIZE(&dl) - 1, sizeof(legh)))
 		return;
 
-	memcpy(&sig, secbuf, sizeof(sig));
-	if (letoh64(sig) == GPTSIGNATURE) {
-		memset(secbuf, 0, dl.d_secsize);
-		if (DISK_writesectors(secbuf, DL_GETDSIZE(&dl) - 1, 1))
+	if (letoh64(legh.gh_sig) == GPTSIGNATURE) {
+		memset(&legh, 0, GPTMINHDRSIZE);
+		if (DISK_writebytes(&legh, DL_GETDSIZE(&dl) - 1, sizeof(legh)))
 			DPRINTF("Unable to zap GPT header @ sector %llu",
 			    DL_GETDSIZE(&dl) - 1);
 	}
-	free(secbuf);
 }
 
 int
 GPT_write(void)
 {
 	struct gpt_header	 legh;
-	char			*secbuf;
 	uint64_t		 altgh, altgp;
 	uint64_t		 gpbytes, gpsectors;
-	int			 rslt;
 
 	if (MBR_write(&gmbr))
 		return -1;
@@ -680,10 +660,6 @@ GPT_write(void)
 	altgh = DL_GETDSIZE(&dl) - 1;
 	altgp = altgh - gpsectors;
 
-	secbuf = DISK_readsectors(GPTSECTOR, 1);
-	if (secbuf == NULL)
-		return -1;
-
 	legh.gh_sig = htole64(GPTSIGNATURE);
 	legh.gh_rev = htole32(GPTREVISION);
 	legh.gh_size = htole32(GPTMINHDRSIZE);
@@ -698,12 +674,10 @@ GPT_write(void)
 	legh.gh_part_num = htole32(gh.gh_part_num);
 	legh.gh_part_size = htole32(GPTMINPARTSIZE);
 	legh.gh_part_csum = htole32(crc32((unsigned char *)&gp, gpbytes));
-
 	legh.gh_csum = htole32(crc32((unsigned char *)&legh, gh.gh_size));
-	memcpy(secbuf, &legh, sizeof(legh));
-	rslt = DISK_writesectors(secbuf, GPTSECTOR, 1);
-	free(secbuf);
-	if (rslt)
+
+	if (DISK_writebytes(&legh, GPTSECTOR, gh.gh_size) ||
+	    DISK_writebytes(&gp, GPTSECTOR + 1, gpbytes))
 		return -1;
 
 	legh.gh_lba_self = htole64(altgh);
@@ -712,19 +686,8 @@ GPT_write(void)
 	legh.gh_csum = 0;
 	legh.gh_csum = htole32(crc32((unsigned char *)&legh, gh.gh_size));
 
-	secbuf = DISK_readsectors(altgh, 1);
-	if (secbuf == NULL)
-		return -1;
-
-	memcpy(secbuf, &legh, gh.gh_size);
-	rslt = DISK_writesectors(secbuf, altgh, 1);
-	free(secbuf);
-	if (rslt)
-		return -1;
-
-	if (DISK_writesectors((const char *)&gp, GPTSECTOR + 1, gpsectors))
-		return -1;
-	if (DISK_writesectors((const char *)&gp, altgp, gpsectors))
+	if (DISK_writebytes(&legh, altgh, gh.gh_size) ||
+	    DISK_writebytes(&gp, altgp, gpbytes))
 		return -1;
 
 	/* Refresh in-kernel disklabel from the updated disk information. */
diff --git a/sbin/fdisk/mbr.c b/sbin/fdisk/mbr.c
index 3cafe10d986..11308d30445 100644
--- a/sbin/fdisk/mbr.c
+++ b/sbin/fdisk/mbr.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: mbr.c,v 1.118 2022/02/04 23:32:17 krw Exp $	*/
+/*	$OpenBSD: mbr.c,v 1.119 2022/04/25 17:10:09 krw Exp $	*/
 
 /*
  * Copyright (c) 1997 Tobias Weingartner
@@ -155,15 +155,10 @@ int
 MBR_read(const uint64_t lba_self, const uint64_t lba_firstembr, struct mbr *mbr)
 {
 	struct dos_mbr		 dos_mbr;
-	char			*secbuf;
 
-	secbuf = DISK_readsectors(lba_self, 1);
-	if (secbuf == NULL)
+	if (DISK_readbytes(&dos_mbr, lba_self, sizeof(dos_mbr)))
 		return -1;
 
-	memcpy(&dos_mbr, secbuf, sizeof(dos_mbr));
-	free(secbuf);
-
 	dos_mbr_to_mbr(&dos_mbr, lba_self, lba_firstembr, mbr);
 
 	return 0;
@@ -173,19 +168,10 @@ int
 MBR_write(const struct mbr *mbr)
 {
 	struct dos_mbr		 dos_mbr;
-	char			*secbuf;
-	int			 rslt;
-
-	secbuf = DISK_readsectors(mbr->mbr_lba_self, 1);
-	if (secbuf == NULL)
-		return -1;
 
 	mbr_to_dos_mbr(mbr, &dos_mbr);
-	memcpy(secbuf, &dos_mbr, sizeof(dos_mbr));
 
-	rslt = DISK_writesectors(secbuf, mbr->mbr_lba_self, 1);
-	free(secbuf);
-	if (rslt)
+	if (DISK_writebytes(&dos_mbr, mbr->mbr_lba_self, sizeof(dos_mbr)))
 		return -1;
 
 	/* Refresh in-kernel disklabel from the updated disk information. */