From adf35c805063bdae2645f1ef12aa003e59773899 Mon Sep 17 00:00:00 2001 From: krw Date: Mon, 25 Apr 2022 17:10:09 +0000 Subject: [PATCH] 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. --- sbin/fdisk/disk.c | 48 ++++++++++++++++++++++++++++--- sbin/fdisk/disk.h | 7 ++--- sbin/fdisk/gpt.c | 73 ++++++++++++----------------------------------- sbin/fdisk/mbr.c | 20 ++----------- 4 files changed, 68 insertions(+), 80 deletions(-) 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 * Copyright (c) 2015 Kenneth R Westerback @@ -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. */ -- 2.20.1