disklib: Change read/write interface

Get rid of the error pointer and use a errno-like error reporting mechanism.
Intent is to make these more like the standard read/write system calls.

Signed-off-by: Pierre-Alexandre Meyer <pierre@mouraf.org>

5 files changed, 109 insertions, 199 deletions

diff --git a/com32/gpllib/disk/errno_disk.c b/com32/gpllib/disk/errno_disk.c
new file mode 100644
index 00000000..8a68802f
--- /dev/null
+++ b/com32/gpllib/disk/errno_disk.c
@@ -0,0 +1,12 @@
+/* ----------------------------------------------------------------------- *
+ *
+ *   Copyright 2009 Pierre-Alexandre Meyer
+ *
+ *   This file is part of Syslinux, and is made available under
+ *   the terms of the GNU General Public License version 2.
+ *
+ * ----------------------------------------------------------------------- */
+
+#include <disk/errno_disk.h>
+
+int errno_disk;
diff --git a/com32/gpllib/disk/error.c b/com32/gpllib/disk/error.c
index 6848ef3e..5491b994 100644
--- a/com32/gpllib/disk/error.c
+++ b/com32/gpllib/disk/error.c
@@ -9,128 +9,16 @@
 
 #include <stdio.h>
 #include <stdlib.h>
-#include <string.h>
+
+#include <disk/errno_disk.h>
 
 /**
  * get_error - decode a disk error status
- * @status:	Error code
- * @buffer_ptr:	Pointer to set to the error message
+ * @buffer_ptr:	Preallocated buffer
  *
- * A buffer will be allocated to contain the error message.
- * @buffer_ptr will point to it. The caller will need to free it.
+ * Fill @buffer_ptr with the last errno_disk
  **/
-void get_error(int status, char** buffer_ptr)
+void get_error(void* buffer_ptr)
 {
-	int buffer_size = (80 * sizeof(char));
-	char* buffer = malloc(buffer_size);
-	*buffer_ptr = buffer;
-
-	switch (status) {
-	case 0x0:
-	strncpy(buffer, "successful completion", buffer_size);
-	break;
-	case 0x01:
-	strncpy(buffer, "invalid function in AH or invalid parameter", buffer_size);
-	break;
-	case 0x02:
-	strncpy(buffer, "address mark not found", buffer_size);
-	break;
-	case 0x03:
-	strncpy(buffer, "disk write-protected", buffer_size);
-	break;
-	case 0x04:
-	strncpy(buffer, "sector not found/read error", buffer_size);
-	break;
-	case 0x05:
-	strncpy(buffer, "reset failed (hard disk)", buffer_size);
-	//strncpy(buffer, "data did not verify correctly (TI Professional PC)", buffer_size);
-	break;
-	case 0x06:
-	strncpy(buffer, "disk changed (floppy)", buffer_size);
-	break;
-	case 0x07:
-	strncpy(buffer, "drive parameter activity failed (hard disk)", buffer_size);
-	break;
-	case 0x08:
-	strncpy(buffer, "DMA overrun", buffer_size);
-	break;
-	case 0x09:
-	strncpy(buffer, "data boundary error (attempted DMA across 64K boundary or >80h sectors)", buffer_size);
-	break;
-	case 0x0A:
-	strncpy(buffer, "bad sector detected (hard disk)", buffer_size);
-	break;
-	case 0x0B:
-	strncpy(buffer, "bad track detected (hard disk)", buffer_size);
-	break;
-	case 0x0C:
-	strncpy(buffer, "unsupported track or invalid media", buffer_size);
-	break;
-	case 0x0D:
-	strncpy(buffer, "invalid number of sectors on format (PS/2 hard disk)", buffer_size);
-	break;
-	case 0x0E:
-	strncpy(buffer, "control data address mark detected (hard disk)", buffer_size);
-	break;
-	case 0x0F:
-	strncpy(buffer, "DMA arbitration level out of range (hard disk)", buffer_size);
-	break;
-	case 0x10:
-	strncpy(buffer, "uncorrectable CRC or ECC error on read", buffer_size);
-	break;
-	case 0x11:
-	strncpy(buffer, "data ECC corrected (hard disk)", buffer_size);
-	break;
-	case 0x20:
-	strncpy(buffer, "controller failure", buffer_size);
-	break;
-	case 0x31:
-	strncpy(buffer, "no media in drive (IBM/MS INT 13 extensions)", buffer_size);
-	break;
-	case 0x32:
-	strncpy(buffer, "incorrect drive type stored in CMOS (Compaq)", buffer_size);
-	break;
-	case 0x40:
-	strncpy(buffer, "seek failed", buffer_size);
-	break;
-	case 0x80:
-	strncpy(buffer, "timeout (not ready)", buffer_size);
-	break;
-	case 0xAA:
-	strncpy(buffer, "drive not ready (hard disk)", buffer_size);
-	break;
-	case 0xB0:
-	strncpy(buffer, "volume not locked in drive (INT 13 extensions)", buffer_size);
-	break;
-	case 0xB1:
-	strncpy(buffer, "volume locked in drive (INT 13 extensions)", buffer_size);
-	break;
-	case 0xB2:
-	strncpy(buffer, "volume not removable (INT 13 extensions)", buffer_size);
-	break;
-	case 0xB3:
-	strncpy(buffer, "volume in use (INT 13 extensions)", buffer_size);
-	break;
-	case 0xB4:
-	strncpy(buffer, "lock count exceeded (INT 13 extensions)", buffer_size);
-	break;
-	case 0xB5:
-	strncpy(buffer, "valid eject request failed (INT 13 extensions)", buffer_size);
-	break;
-	case 0xBB:
-	strncpy(buffer, "undefined error (hard disk)", buffer_size);
-	break;
-	case 0xCC:
-	strncpy(buffer, "write fault (hard disk)", buffer_size);
-	break;
-	case 0xE0:
-	strncpy(buffer, "status register error (hard disk)", buffer_size);
-	break;
-	case 0xFF:
-	strncpy(buffer, "sense operation failed (hard disk)", buffer_size);
-	break;
-	default:
-	snprintf(buffer, buffer_size, "unknown error 0x%X, buggy bios?", status);
-	break;
-	}
+	sprintf(buffer_ptr, "Disklib: error %d\n", errno_disk);
 }
diff --git a/com32/gpllib/disk/msdos.c b/com32/gpllib/disk/msdos.c
index 3b31dd0d..249d39c4 100644
--- a/com32/gpllib/disk/msdos.c
+++ b/com32/gpllib/disk/msdos.c
@@ -13,6 +13,8 @@
  * ----------------------------------------------------------------------- */
 
 #include <stdlib.h>
+
+#include <disk/common.h>
 #include <disk/geom.h>
 #include <disk/partition.h>
 #include <disk/read.h>
@@ -23,7 +25,7 @@ static inline int is_extended_partition(struct part_entry *ptab)
 		ptab->ostype == 0x0f ||
 		ptab->ostype == 0x85);
 }
-static inline int msdos_magic_present(char *ptab)
+static inline int msdos_magic_present(const char *ptab)
 {
 	return ( *(uint16_t *)(ptab + 0x1fe) == 0xaa55 );
 }
@@ -36,25 +38,28 @@ static inline int msdos_magic_present(char *ptab)
  * @error:		Buffer for I/O errors
  * @nb_part_seen:	Number of partitions found on the disk so far
  **/
-static void process_extended_partition(struct driveinfo *drive_info,
+static int process_extended_partition(struct driveinfo *drive_info,
 				       int partition_offset,
 				       void *callback(struct driveinfo *, struct part_entry *, int, int),
-				       int *error, int nb_part_seen)
+				       int nb_part_seen)
 {
+	int status = 0;
 	/* The ebr is located at the first sector of the extended partition */
-	char* ebr = read_sectors(drive_info, partition_offset, 1, error);
+	char* ebr = malloc(SECTOR * sizeof(char));
 
-	/* If something bad during the read happens, we can't do much: bail out */
-	if (*error)
-		return;
+	if (read_sectors(drive_info, ebr, partition_offset, 1) == -1)
+		return -1;
 
 	/* Check msdos magic signature */
 	if (!msdos_magic_present(ebr))
-		return;
+		return -1;
 
 	struct part_entry *ptab = (struct part_entry *)(ebr + PARTITION_TABLES_OFFSET);
 
 	for (int i = 0; i < 4; i++) {
+		if (status == -1)
+			return -1;
+
 		if (!is_extended_partition(&ptab[i])) {
 			/*
 			 * This EBR partition table entry points to the
@@ -81,11 +86,13 @@ static void process_extended_partition(struct driveinfo *drive_info,
 				 logical_partition_start,
 				 nb_part_seen);
 		} else
-			process_extended_partition(drive_info,
+			status = process_extended_partition(drive_info,
 						   partition_offset + ptab[i].start_lba,
 						   callback,
-						   error, nb_part_seen);
+						   nb_part_seen);
 	}
+
+	return 0;
 }
 
 /**
@@ -93,15 +100,15 @@ static void process_extended_partition(struct driveinfo *drive_info,
  * @drive_info:	driveinfo struct describing the drive
  * @ptab:	Pointer to the partition table
  * @callback:	Callback to execute
- * @error:	Return the error code (I/O), if needed
  **/
-static void process_mbr(struct driveinfo *drive_info, struct part_entry *ptab,
-		        void *callback(struct driveinfo *, struct part_entry *, int, int),
-		        int *error)
+static int process_mbr(struct driveinfo *drive_info, struct part_entry *ptab,
+		       void *callback(struct driveinfo *, struct part_entry *, int, int))
 {
+	int status = 0;
+
 	for (int i = 0; i < 4; i++) {
-		if (*error)
-			return;
+		if (status == -1)
+			return -1;
 
 		if (ptab[i].start_sect > 0) {
 			if (is_extended_partition(&ptab[i])) {
@@ -109,7 +116,7 @@ static void process_mbr(struct driveinfo *drive_info, struct part_entry *ptab,
 					 &ptab[i],
 					 ptab[i].start_lba,
 					 i+1);
-				process_extended_partition(drive_info, ptab[i].start_lba, callback, error, 4);
+				status = process_extended_partition(drive_info, ptab[i].start_lba, callback, 4);
 			} else
 				callback(drive_info,
 					 &ptab[i],
@@ -117,6 +124,8 @@ static void process_mbr(struct driveinfo *drive_info, struct part_entry *ptab,
 					 i+1);
 		}
 	}
+
+	return 0;
 }
 
 /**
@@ -132,10 +141,11 @@ static void process_mbr(struct driveinfo *drive_info, struct part_entry *ptab,
  *		 int offset_root,
  *		 int nb_part_seen)
  **/
-int parse_partition_table(struct driveinfo *d, void *callback, int *error)
+int parse_partition_table(struct driveinfo *d, void *callback)
 {
-	char *mbr = read_mbr(d->disk, error);
-	if (!mbr)
+	char *mbr = malloc(SECTOR * sizeof(char));
+
+	if (read_mbr(d->disk, mbr) == -1)
 		return -1;
 	else {
 		/* Check msdos magic signature */
@@ -143,10 +153,6 @@ int parse_partition_table(struct driveinfo *d, void *callback, int *error)
 			return -1;
 
 		struct part_entry *ptab = (struct part_entry *)(mbr + PARTITION_TABLES_OFFSET);
-		process_mbr(d, ptab, callback, error);
-		if (*error)
-			return -1;
-		else
-			return 0;
+		return process_mbr(d, ptab, callback);
 	}
 }
diff --git a/com32/gpllib/disk/read.c b/com32/gpllib/disk/read.c
index 3fc0e0c7..74840829 100644
--- a/com32/gpllib/disk/read.c
+++ b/com32/gpllib/disk/read.c
@@ -16,61 +16,72 @@
 #include <stdlib.h>
 #include <string.h>
 
+#include <disk/errno_disk.h>
 #include <disk/geom.h>
 #include <disk/read.h>
 #include <disk/util.h>
 #include <disk/common.h>
 
+/*
+ * TODO: implement file descriptors to cache metadata (geometry, ...)
+ */
+
 /**
  * read_mbr - return a pointer to a malloced buffer containing the mbr
  * @drive:	Drive number
- * @error:	Return the error code on failure
+ * @buf:	Pre-allocated buffer for output
+ *
+ * Return the number of sectors read on success or -1 on failure.
+ * errno_disk contains the error number.
  **/
-void *read_mbr(int drive, int *error)
+int read_mbr(int drive, void *buf)
 {
 	struct driveinfo drive_info;
 	drive_info.disk = drive;
 
 	/* MBR: lba = 0, 1 sector */
-	return read_sectors(&drive_info, 0, 1, error);
+	return read_sectors(&drive_info, buf, 0, 1);
 }
 
 /**
  * dev_read - read from a drive
  * @drive:	Drive number
+ * @buf:	Pre-allocated buffer for output
  * @lba:	Position to start reading from
  * @sectors:	Number of sectors to read
- * @error:	Return the error code on failure
  *
  * High-level routine to read from a hard drive.
+ * Return the number of sectors read on success or -1 on failure.
+ * errno_disk contains the error number.
  **/
-void *dev_read(int drive, unsigned int lba, int sectors, int *error)
+int dev_read(int drive, void * buf, unsigned int lba, int sectors)
 {
 	struct driveinfo drive_info;
 	drive_info.disk = drive;
 
-	return read_sectors(&drive_info, lba, sectors, error);
+	return read_sectors(&drive_info, buf, lba, sectors);
 }
 
 /**
  * read_sectors - read several sectors from disk
  * @drive_info:		driveinfo struct describing the disk
+ * @data:		Pre-allocated buffer for output
  * @lba:		Position to read
  * @sectors:		Number of sectors to read
- * @error:		Return the error code on failure
  *
- * Return a pointer to a malloc'ed buffer containing the data.
+ * Return the number of sectors read on success or -1 on failure.
+ * errno_disk contains the error number.
  **/
-void *read_sectors(struct driveinfo* drive_info, const unsigned int lba,
-		   const int sectors, int *error)
+int read_sectors(struct driveinfo* drive_info, void *data,
+		 const unsigned int lba, const int sectors)
 {
 	com32sys_t inreg, outreg;
 	struct ebios_dapa *dapa = __com32.cs_bounce;
 	void *buf = (char *)__com32.cs_bounce + sectors * SECTOR;
-	void *data;
+	char *bufp = data;
 
 	if (get_drive_parameters(drive_info) == -1)
-		return NULL;
+		return -1;
 
 	memset(&inreg, 0, sizeof inreg);
 
@@ -88,17 +99,18 @@ void *read_sectors(struct driveinfo* drive_info, const unsigned int lba,
 	} else {
 		unsigned int c, h, s;
 
-		if (!drive_info->cbios) {
+		if (!drive_info->cbios) { // XXX errno
 			/* We failed to get the geometry */
 			if (lba)
-				return NULL;	/* Can only read MBR */
+				return -1;	/* Can only read MBR */
 
 			s = 1;  h = 0;  c = 0;
 		} else
 			lba_to_chs(drive_info, lba, &s, &h, &c);
 
+		// XXX errno
 		if ( s > 63 || h > 256 || c > 1023 )
-			return NULL;
+			return -1;
 
 		inreg.eax.w[0] = 0x0201;	/* Read one sector */
 		inreg.ecx.b[1] = c & 0xff;
@@ -111,17 +123,11 @@ void *read_sectors(struct driveinfo* drive_info, const unsigned int lba,
 
 	/* Perform the read */
 	if (int13_retry(&inreg, &outreg)) {
-		if (error)
-			*error = outreg.eax.b[1];
-		return NULL;	/* Give up */
-	} else {
-		if (error)
-			*error = 0;
+		errno_disk = outreg.eax.b[1];
+		return -1;	/* Give up */
 	}
 
-	data = malloc(sectors * SECTOR);
-	if (data)
-		memcpy(data, buf, sectors * SECTOR);
+	memcpy(bufp, buf, sectors * SECTOR);
 
-	return data;
+	return sectors;
 }
diff --git a/com32/gpllib/disk/write.c b/com32/gpllib/disk/write.c
index b772ba72..a236f37d 100644
--- a/com32/gpllib/disk/write.c
+++ b/com32/gpllib/disk/write.c
@@ -15,10 +15,12 @@
 #include <com32.h>
 #include <stdlib.h>
 #include <string.h>
-#include <disk/read.h>
-#include <disk/write.h>
+
 #include <disk/common.h>
+#include <disk/errno_disk.h>
+#include <disk/read.h>
 #include <disk/util.h>
+#include <disk/write.h>
 
 /**
  * write_sectors - write several sectors from disk
@@ -26,10 +28,12 @@
  * @lba:		Position to write
  * @data:		Buffer to write
  * @size:		Size of the buffer (number of sectors)
- * @error:		Return the error code on failure
+ *
+ * Return the number of sectors write on success or -1 on failure.
+ * errno_disk contains the error number.
  **/
 int write_sectors(const struct driveinfo* drive_info, const unsigned int lba,
-		  const void *data, const int size, int *error)
+		  const void *data, const int size)
 {
 	com32sys_t inreg, outreg;
 	struct ebios_dapa *dapa = __com32.cs_bounce;
@@ -38,7 +42,7 @@ int write_sectors(const struct driveinfo* drive_info, const unsigned int lba,
 	memcpy(buf, data, size);
 	memset(&inreg, 0, sizeof inreg);
 
-	if ( drive_info->ebios ) {
+	if (drive_info->ebios) {
 		dapa->len = sizeof(*dapa);
 		dapa->count = size;
 		dapa->off = OFFS(buf);
@@ -52,16 +56,16 @@ int write_sectors(const struct driveinfo* drive_info, const unsigned int lba,
 	} else {
 		unsigned int c, h, s;
 
-		if ( !drive_info->cbios ) {
-		  /* We failed to get the geometry */
-
-		  if ( lba )
-		    return -1;		/* Can only write MBR */
+		if (!drive_info->cbios) { // XXX errno
+			/* We failed to get the geometry */
+			if (lba)
+				return -1;	/* Can only write MBR */
 
-		  s = 1;  h = 0;  c = 0;
+			s = 1;  h = 0;  c = 0;
 		} else
 		    lba_to_chs(drive_info, lba, &s, &h, &c);
 
+		// XXX errno
 		if ( s > 63 || h > 256 || c > 1023 )
 		  return -1;
 
@@ -76,14 +80,10 @@ int write_sectors(const struct driveinfo* drive_info, const unsigned int lba,
 
 	/* Perform the write */
 	if (int13_retry(&inreg, &outreg)) {
-		if (error)
-			*error = outreg.eax.b[1];
+		errno_disk = outreg.eax.b[1];
 		return -1;	/* Give up */
-	} else {
-		if (error)
-			*error = 0;
-		return 0;
-	}
+	} else
+		return size;
 }
 
 /**
@@ -94,9 +94,9 @@ int write_sectors(const struct driveinfo* drive_info, const unsigned int lba,
  **/
 int write_verify_sector(struct driveinfo* drive_info,
 			const unsigned int lba,
-			const void *data, int *error)
+			const void *data)
 {
-	return write_verify_sectors(drive_info, lba, data, SECTOR, error);
+	return write_verify_sectors(drive_info, lba, data, SECTOR);
 }
 
 /**
@@ -108,20 +108,18 @@ int write_verify_sector(struct driveinfo* drive_info,
  **/
 int write_verify_sectors(struct driveinfo* drive_info,
 			 const unsigned int lba,
-			 const void *data, const int size, int* error)
+			 const void *data, const int size)
 {
-	char *rb;
-	int rv;
+	char *rb = malloc(SECTOR * size * sizeof(char));
+	int status;
 
-	rv = write_sectors(drive_info, lba, data, size, error);
-	if (rv)
-		return rv;		/* Write failure */
+	if (write_sectors(drive_info, lba, data, size) == -1)
+		return -1;	/* Write failure */
 
-	rb = read_sectors(drive_info, lba, size, error);
-	if (!rb)
-		return -1;		/* Readback failure */
+	if (read_sectors(drive_info, rb, lba, size) == -1)
+		return -1;	/* Readback failure */
 
-	rv = memcmp(data, rb, SECTOR * size);
+	status = memcmp(data, rb, SECTOR * size);
 	free(rb);
-	return rv ? -1 : 0;
+	return status ? -1 : 0;
 }