[patch] bugs in libstore and storeio

[Neal H Walfield]

There are several bugs in libstore and storeio.  In fact, it appears
that a lot of the code has never actually been used.  Anyway:

 o vm_write expects that the buffer to flush, in addition to the page
   and the length, be page aligned.  Refer to Mach 3 Kernel for the
   whole story.

 o We need to be careful with lengths; sometimes, they are in bytes
   and sometimes, they are in pages.

 o Initialization.

Here is a change log for libstore/ChangeLog:

2001-08-12  Neal H Walfield  <neal@cs.uml.edu>

        * copy.c (copy_read): The protocol dictates that *LEN is in
        bytes, not pages.
        (copy_write): Be sure that the buffer passed to vm_read is page
        aligned.
        When determining how much to copy, use LEN, not the uninitialized
        *AMOUNT.
        (copy_clone): Use memcpy, not bcopy.
        * derive.c: Include <sys/types.h> and <mach.h>.
        (_store_derive): Initialize STORE->wrap_src.

        * set.c: Do not include <malloc.h>.
        Include <stdlib.h>, <errno.h> and <mach.h>.
        (store_set_runs): Use memcpy, not bcopy.
        (store_set_name): Use strdup, not a strlen, malloc and strcpy.
        * zero.c (zero_read): When checking if mmap failed, compare
        against MAP_FAILED, not -1.
        Use memset, not bzero.

And storeio/ChangeLog:

2001-08-12  Neal H Walfield  <neal@cs.uml.edu>

        * pager.c: Include <errno.h>.
        (pager_read_page): Use memset, not bzero.
        * storeio.c (check_open_hook): Doc fix.
        (trivfs_modify_stat): STORE->size is a store_offset_t.  It not a
        vm_size_t.


Index: libstore/copy.c
===================================================================
RCS file: /cvs/hurd/libstore/copy.c,v
retrieving revision 1.9
diff -u -p -r1.9 copy.c
--- libstore/copy.c     2001/03/07 15:01:37     1.9
+++ libstore/copy.c     2001/08/14 10:30:46
@@ -30,8 +30,8 @@
 #include "store.h"
 
 static error_t
-copy_read (struct store *store,
-          store_offset_t addr, size_t index, size_t amount, void **buf, size_t 
*len)
+copy_read (struct store *store, store_offset_t addr, size_t index,
+          size_t amount, void **buf, size_t *len)
 {
   char *data = store->hook + (addr * store->block_size);
 
@@ -39,7 +39,9 @@ copy_read (struct store *store,
     {
       /* When reading whole pages, we can avoid any real copying.  */
       error_t err = vm_read (mach_task_self (),
-                            (vm_address_t) data, amount, (pointer_t *) buf, 
len);
+                            (vm_address_t) data, amount,
+                            (pointer_t *) buf, len);
+      *len *= vm_page_size;
       return err;
     }
 
@@ -63,16 +65,16 @@ copy_write (struct store *store,
 {
   char *data = store->hook + (addr * store->block_size);
 
-  if (page_aligned (data) && page_aligned (len))
+  if (page_aligned (data) && page_aligned (len) && page_aligned (buf))
     {
-      /* When reading whole pages, we can avoid any real copying.  */
+      /* When writing whole pages, we can avoid any real copying.  */
       error_t err = vm_write (mach_task_self (),
                              (vm_address_t) data, (vm_address_t) buf, len);
       *amount = len;
       return err;
     }
 
-  memcpy (data, buf, *amount);
+  memcpy (data, buf, len);
   *amount = len;
   return 0;
 }
@@ -152,7 +154,7 @@ copy_clone (const struct store *from, st
   if (buf != (void *) -1)
     {
       to->hook = buf;
-      bcopy (from->hook, to->hook, from->size);
+      memcpy (to->hook, from->hook, from->size);
       return 0;
     }
   return errno;

Index: libstore/derive.c
===================================================================
RCS file: /cvs/hurd/libstore/derive.c,v
retrieving revision 1.10
diff -u -p -r1.10 derive.c
--- libstore/derive.c   2001/01/08 22:33:11     1.10
+++ libstore/derive.c   2001/08/14 10:30:46
@@ -1,7 +1,7 @@
 /* Calculation of various derived store fields
 
-   Copyright (C) 1995, 1996, 1997 Free Software Foundation, Inc.
-   Written by Miles Bader <miles@gnu.ai.mit.edu>
+   Copyright (C) 1995-97,2001 Free Software Foundation, Inc.
+   Written by Miles Bader <miles@gnu.org>
    This file is part of the GNU Hurd.
 
    The GNU Hurd is free software; you can redistribute it and/or
@@ -19,6 +19,8 @@
    Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111, USA. */
 
 #include <assert.h>
+#include <sys/types.h>
+#include <mach.h>
 
 #include "store.h"
 
@@ -34,6 +36,7 @@ _store_derive (struct store *store)
 
   /* BLOCK & SIZE */
   store->blocks = 0;
+  store->wrap_src = 0;
 
   for (i = 0; i < num_runs; i++)
     {
Index: libstore/set.c
===================================================================
RCS file: /cvs/hurd/libstore/set.c,v
retrieving revision 1.10
diff -u -p -r1.10 set.c
--- libstore/set.c      2001/05/06 13:18:28     1.10
+++ libstore/set.c      2001/08/14 10:30:47
@@ -1,7 +1,7 @@
 /* Setting various store fields
 
-   Copyright (C) 1995, 1996, 1997 Free Software Foundation, Inc.
-   Written by Miles Bader <miles@gnu.ai.mit.edu>
+   Copyright (C) 1995, 1996, 1997, 2001 Free Software Foundation, Inc.
+   Written by Miles Bader <miles@gnu.org>
    This file is part of the GNU Hurd.
 
    The GNU Hurd is free software; you can redistribute it and/or
@@ -18,8 +18,10 @@
    along with this program; if not, write to the Free Software
    Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111, USA. */
 
-#include <malloc.h>
+#include <stdlib.h>
 #include <string.h>
+#include <errno.h>
+#include <mach.h>
 
 #include "store.h"
 
@@ -37,7 +39,7 @@ store_set_runs (struct store *store,
   if (store->runs)
     free (store->runs);
 
-  bcopy (runs, copy, size);
+  memcpy (copy, runs, size);
   store->runs = copy;
   store->num_runs = num_runs;
 
@@ -51,7 +53,7 @@ store_set_runs (struct store *store,
 error_t
 store_set_name (struct store *store, const char *name)
 {
-  char *copy = malloc (strlen (name) + 1);
+  char *copy = strdup (name);
 
   if (!copy)
     return ENOMEM;
@@ -59,7 +61,6 @@ store_set_name (struct store *store, con
   if (store->name)
     free (store->name);
 
-  strcpy (copy, name);
   store->name = copy;
 
   return 0;
Index: libstore/zero.c
===================================================================
RCS file: /cvs/hurd/libstore/zero.c,v
retrieving revision 1.11
diff -u -p -r1.11 zero.c
--- libstore/zero.c     2001/03/07 15:01:37     1.11
+++ libstore/zero.c     2001/08/14 10:30:49
@@ -35,13 +35,13 @@ zero_read (struct store *store,
   if (*len < amount)
     {
       *buf = mmap (0, amount, PROT_READ|PROT_WRITE, MAP_ANON, 0, 0);
-      if (*buf == (void *) -1)
+      if (*buf == MAP_FAILED)
        return errno;
       *len = amount;
       return 0;
     }
   else
-    bzero (*buf, amount);
+    memset (*buf, 0, amount);
 
   *len = amount;
   return 0;
Index: storeio/pager.c
===================================================================
RCS file: /cvs/hurd/storeio/pager.c,v
retrieving revision 1.11
diff -u -p -r1.11 pager.c
--- storeio/pager.c     1999/07/11 01:39:11     1.11
+++ storeio/pager.c     2001/08/14 10:30:52
@@ -23,6 +23,7 @@
 #include <assert.h>
 #include <strings.h>
 #include <unistd.h>
+#include <errno.h>
 #include <sys/mman.h>
 
 #include "dev.h"
@@ -53,7 +54,7 @@ pager_read_page (struct user_pager_info 
   if (!err && want < vm_page_size)
     /* Zero anything we didn't read.  Allocation only happens in page-size
        multiples, so we know we can write there.  */
-    bzero ((char *)*buf + want, vm_page_size - want);
+    memset ((char *)*buf + want, '\0', vm_page_size - want);
 
   *writelock = (store->flags & STORE_READONLY);
 
Index: storeio/storeio.c
===================================================================
RCS file: /cvs/hurd/storeio/storeio.c,v
retrieving revision 1.21
diff -u -p -r1.21 storeio.c
--- storeio/storeio.c   2001/04/19 22:11:52     1.21
+++ storeio/storeio.c   2001/08/14 10:30:53
@@ -222,7 +222,7 @@ check_open_hook (struct trivfs_control *
       err = dev_open (dev);
       if (err && (flags & (O_READ|O_WRITE)) == 0)
        /* If we're not opening for read or write, then just ignore the
-          error, as this allows stat to word correctly.  XXX  */
+          error, as this allows stat to work correctly.  XXX  */
        err = 0;
     }
   mutex_unlock (&dev->lock);
@@ -287,7 +287,7 @@ trivfs_modify_stat (struct trivfs_protid
     /* An open device.  */
     {
       struct store *store = open->dev->store;
-      vm_size_t size = store->size;
+      store_offset_t size = store->size;
 
       if (store->block_size > 1)
        st->st_blksize = store->block_size;

pgpYvg22VV_AG.pgp
Description: PGP signature