Syslinux - Apr 2010 - gpllib write_sectors() patch

Here is a partial patch that I'm using to illustrate a few questions
that I have.

Looking at the gpllib write_sectors (com32/gpllib/disk/write.c) it
looks like the allocated
size is wrong; size is in sectors so we should adjust the alloc/memcpy
by size*SECTOR.

Also, comparing core/diskio.c it looks like the registers aren't even
setup right.
In the patch below I put an #if 0 around the old code to demonstrate
how it differs
from diskio.c

I also added a call to get_drive_parameters() so that it would be like
the gpllib/read.c
However, it would probably be best to not have to call
get_drive_parameters every time
so they should be supplied in the driveinfo struct (for both read and write).

 /**
  * write_sectors - write several sectors from disk
@@ -37,9 +55,14 @@ int write_sectors(const struct driveinfo
*drive_info, const unsigned int lba,
 {
     com32sys_t inreg, outreg;
     struct ebios_dapa *dapa = __com32.cs_bounce;
-    void *buf = (char *)__com32.cs_bounce + size;
+    void *buf = (char *)__com32.cs_bounce + size*SECTOR;

-    memcpy(buf, data, size);
+    if (get_drive_parameters(drive_info) == -1) {
+      printf("ERR: Can't get drive params!\n");
+      return -1;
+    }
+
+    memcpy(buf, data, size*SECTOR);
     memset(&inreg, 0, sizeof inreg);

     if (drive_info->ebios) {
@@ -48,11 +71,17 @@ int write_sectors(const struct driveinfo
*drive_info, const unsigned int lba,
        dapa->off = OFFS(buf);
        dapa->seg = SEG(buf);
        dapa->lba = lba;
-
+#if 0
        inreg.esi.w[0] = OFFS(dapa);
        inreg.ds = SEG(dapa);
        inreg.edx.b[0] = drive_info->disk;
        inreg.eax.w[0] = 0x4300;        /* Extended write */
+#endif
+       inreg.eax.b[1] = 0x43;
+       inreg.edx.b[0] = drive_info->disk;
+       inreg.ds       = SEG(dapa);
+       inreg.esi.w[0] = OFFS(dapa);
+

On 04/27/2010 10:15 AM, Don Hiatt wrote:
> Here is a partial patch that I'm using to illustrate a few questions
> that I have.
> 
> Looking at the gpllib write_sectors (com32/gpllib/disk/write.c) it
> looks like the allocated
> size is wrong; size is in sectors so we should adjust the alloc/memcpy
> by size*SECTOR.
> 
> Also, comparing core/diskio.c it looks like the registers aren't even
> setup right. In the patch below I put an #if 0 around the old code to
demonstrate
> how it differs from diskio.c
In doing that, you rather defeat the purpose of a diff, and so it's a
lot harder to read.
>         dapa->lba = lba;
> -
> +#if 0
>         inreg.esi.w[0] = OFFS(dapa);
>         inreg.ds = SEG(dapa);
>         inreg.edx.b[0] = drive_info->disk;
>         inreg.eax.w[0] = 0x4300;        /* Extended write */
> +#endif
> +       inreg.eax.b[1] = 0x43;
> +       inreg.edx.b[0] = drive_info->disk;
> +       inreg.ds       = SEG(dapa);
> +       inreg.esi.w[0] = OFFS(dapa);
> +
> 
The only difference (other than a gratuitous reshuffling) that I see is
the value of AL, which *should* be cleared: AL is a subfunction
identifier for type 0x43.

	-hpa

- Fixes the buffer size in write.
- Adds a dev_write() to compliment dev_read()
- Adds a call to get_drive_parameters to be similar to read_sectors()

diff --git a/com32/gplinclude/disk/write.h b/com32/gplinclude/disk/write.h
index 89d26fc..407d660 100644
--- a/com32/gplinclude/disk/write.h
+++ b/com32/gplinclude/disk/write.h
@@ -16,7 +16,7 @@
 #define _WRITE_H_

 #include <disk/geom.h>
-
+int dev_write(int, void *, unsigned int, int);
 int write_sectors(const struct driveinfo *, const unsigned int,
 		  const void *, const int);
 int write_verify_sector(struct driveinfo *drive_info,
diff --git a/com32/gpllib/disk/write.c b/com32/gpllib/disk/write.c
index 89e530d..2eaa189 100644
--- a/com32/gpllib/disk/write.c
+++ b/com32/gpllib/disk/write.c
@@ -23,6 +23,25 @@
 #include <disk/write.h>

 /**
+ * dev_write - write to a drive
+ * @drive:	Drive number
+ * @buf:	Pre-allocated buffer for output
+ * @lba:	Position to start writing to
+ * @sectors:	Number of sectors to write
+ *
+ * High-level routine to read from a hard drive.
+ * Return the number of sectors written on success or -1 on failure.
+ * errno_disk contains the error number.
+ **/
+int dev_write(int drive, void *buf, unsigned int lba, int sectors)
+{
+    struct driveinfo drive_info;
+    drive_info.disk = drive;
+
+    return write_sectors(&drive_info, lba, buf, sectors);
+}
+
+/**
  * write_sectors - write several sectors from disk
  * @drive_info:		driveinfo struct describing the disk
  * @lba:		Position to write
@@ -37,9 +56,13 @@ int write_sectors(const struct driveinfo
*drive_info, const unsigned int lba,
 {
     com32sys_t inreg, outreg;
     struct ebios_dapa *dapa = __com32.cs_bounce;
-    void *buf = (char *)__com32.cs_bounce + size;
+    void *buf = (char *)__com32.cs_bounce + (size * SECTOR);
+
+    if (get_drive_parameters(drive_info) == -1) {
+      return -1;
+    }

-    memcpy(buf, data, size);
+    memcpy(buf, data, (size * SECTOR));
     memset(&inreg, 0, sizeof inreg);

     if (drive_info->ebios) {