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 c505a87..ddcd0c3 100644 (file)
--- 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 84de2a3..7daf790 100644 (file)
--- 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 608e282..ac693bc 100644 (file)
--- 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 3cafe10..11308d3 100644 (file)
--- 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. */