2005-07-20 Yoshinori K. Okuji <okuji@enbug.org>

Change the semantics of variable hooks. They now return	strings
	instead of error values.

	* util/i386/pc/grub-setup.c: Include grub/env.h.
	(setup): Use grub_device_set_root instead of grub_env_set.

	* kern/rescue.c (grub_rescue_cmd_root): Use grub_env_set and
	grub_env_get instead of grub_device_set_root and
	grub_device_get_root, respectively.

	* kern/main.c (grub_env_write_root): New function.
	(grub_set_root_dev): Register grub_env_write_hook for "root". Use
	grub_env_set instead of grub_device_set_root.

	* kern/env.c (HASHSZ): Reduced to 13, because GRUB does not need
	many variables.
	(grub_env_set): Set ENV->VALUE to the result of ENV->WRITE_HOOK
	rather than calling ENV->WRITE_HOOK afterwards.
	(grub_env_get): Return the result of ENV->READ_HOOK rather than
	passing a pointer of a pointer.
	(grub_register_variable_hook): Change the types of "read_hook" and
	"write_hook" to grub_env_read_hook_t and grub_env_write_hook_t,
	respectively.
	Allocate the default empty string on the heap, because this string
	may be freed later.

	* kern/device.c: Include grub/env.h.
	(grub_device_set_root): Removed.
	(grub_device_get_root): Likewise.
	(grub_device_open): Use grub_env_get instead of
	grub_device_get_root.

	* include/grub/env.h (grub_env_read_hook_t): New type.
	(grub_env_write_hook_t): Likewise.
	(grub_env_var): Change the types of "read_hook" and "write_hook"
	to grub_env_read_hook_t and grub_env_write_hook_t, respectively.
	(grub_register_variable_hook): Likewise.

	* include/grub/device.h (grub_device_set_root): Removed.
	(grub_device_set_root): Likewise.

	* fs/fat.c (grub_fat_dir): Make a copy of PATH in DIRNAME, and
	make sure that DIRNAME terminates with '/', so that
	grub_fat_find_dir will fail if PATH is not a directory.

	* commands/ls.c (grub_ls_list_files): Remove the qualifier const
	from DIRNAME.
	Use the qualifier auto for print_files and print_files_long.
	If FS->DIR sets GRUB_ERRNO to GRUB_ERR_BAD_FILE_TYPE, try DIRNAME
	as a regular file.
	Put a newline only if there is no error.
	(grub_cmd_ls): Remove grub_ls_print_files, because this is not
	used.

ChangeLog

@@ -1,3 +1,59 @@
+2005-07-20  Yoshinori K. Okuji  <okuji@enbug.org>
+
+	Change the semantics of variable hooks. They now return	strings
+	instead of error values.
+	
+	* util/i386/pc/grub-setup.c: Include grub/env.h.
+	(setup): Use grub_device_set_root instead of grub_env_set.
+
+	* kern/rescue.c (grub_rescue_cmd_root): Use grub_env_set and
+	grub_env_get instead of grub_device_set_root and
+	grub_device_get_root, respectively.
+
+	* kern/main.c (grub_env_write_root): New function.
+	(grub_set_root_dev): Register grub_env_write_hook for "root". Use
+	grub_env_set instead of grub_device_set_root.
+
+	* kern/env.c (HASHSZ): Reduced to 13, because GRUB does not need
+	many variables.
+	(grub_env_set): Set ENV->VALUE to the result of ENV->WRITE_HOOK
+	rather than calling ENV->WRITE_HOOK afterwards.
+	(grub_env_get): Return the result of ENV->READ_HOOK rather than
+	passing a pointer of a pointer.
+	(grub_register_variable_hook): Change the types of "read_hook" and
+	"write_hook" to grub_env_read_hook_t and grub_env_write_hook_t,
+	respectively.
+	Allocate the default empty string on the heap, because this string
+	may be freed later.
+
+	* kern/device.c: Include grub/env.h.
+	(grub_device_set_root): Removed.
+	(grub_device_get_root): Likewise.
+	(grub_device_open): Use grub_env_get instead of
+	grub_device_get_root.
+
+	* include/grub/env.h (grub_env_read_hook_t): New type.
+	(grub_env_write_hook_t): Likewise.
+	(grub_env_var): Change the types of "read_hook" and "write_hook"
+	to grub_env_read_hook_t and grub_env_write_hook_t, respectively.
+	(grub_register_variable_hook): Likewise.
+
+	* include/grub/device.h (grub_device_set_root): Removed.
+	(grub_device_set_root): Likewise.
+
+	* fs/fat.c (grub_fat_dir): Make a copy of PATH in DIRNAME, and
+	make sure that DIRNAME terminates with '/', so that
+	grub_fat_find_dir will fail if PATH is not a directory.
+
+	* commands/ls.c (grub_ls_list_files): Remove the qualifier const
+	from DIRNAME.
+	Use the qualifier auto for print_files and print_files_long.
+	If FS->DIR sets GRUB_ERRNO to GRUB_ERR_BAD_FILE_TYPE, try DIRNAME
+	as a regular file.
+	Put a newline only if there is no error.
+	(grub_cmd_ls): Remove grub_ls_print_files, because this is not
+	used.
+
 2005-07-20  Yoshinori K. Okuji  <okuji@enbug.org>
 
 	* kern/partition.c (grub_partition_probe): Initialize PART to

commands/ls.c

@@ -121,14 +121,16 @@ grub_ls_list_disks (int longlist)
 }
 
 static grub_err_t
-grub_ls_list_files (const char *dirname, int longlist, int all, int human)
+grub_ls_list_files (char *dirname, int longlist, int all, int human)
 {
   char *device_name;
   grub_fs_t fs;
   const char *path;
   grub_device_t dev;
+  auto int print_files (const char *filename, int dir);
+  auto int print_files_long (const char *filename, int dir);
   
-  static int print_files (const char *filename, int dir)
+  int print_files (const char *filename, int dir)
     {
       if (all || filename[0] != '.')
 	grub_printf ("%s%s ", filename, dir ? "/" : "");
@@ -136,7 +138,7 @@ grub_ls_list_files (const char *dirname, int longlist, int all, int human)
       return 0;
     }
      
-  static int print_files_long (const char *filename, int dir)
+  int print_files_long (const char *filename, int dir)
     {
       char pathname[grub_strlen (dirname) + grub_strlen (filename) + 1];
 
@@ -228,7 +230,39 @@ grub_ls_list_files (const char *dirname, int longlist, int all, int human)
 	(fs->dir) (dev, path, print_files_long);
       else
 	(fs->dir) (dev, path, print_files);
+
+      if (grub_errno == GRUB_ERR_BAD_FILE_TYPE
+	  && path[grub_strlen (path) - 1] != '/')
+	{
+	  /* PATH might be a regular file.  */
+	  char *p;
+	  grub_file_t file;
+
+	  grub_errno = 0;
+	  
+	  file = grub_file_open (dirname);
+	  if (! file)
+	    goto fail;
+	  
+	  grub_file_close (file);
+	  
+	  p = grub_strrchr (dirname, '/') + 1;
+	  dirname = grub_strndup (dirname, p - dirname);
+	  if (! dirname)
+	    goto fail;
+
+	  all = 1;
+	  if (longlist)
+	    print_files_long (p, 0);
+	  else
+	    print_files (p, 0);
+
+	  grub_free (dirname);
+	}
+
+      if (grub_errno == GRUB_ERR_NONE)
 	grub_putchar ('\n');
+      
       grub_refresh ();
     }
 
@@ -244,14 +278,6 @@ grub_ls_list_files (const char *dirname, int longlist, int all, int human)
 static grub_err_t
 grub_cmd_ls (struct grub_arg_list *state, int argc, char **args)
 {
-  static int grub_ls_print_files (const char *filename, int dir)
-    {
-      if (state[2].set/*all*/ || filename[0] != '.')
-	grub_printf ("%s%s ", filename, dir ? "/" : "");
-      
-      return 0;
-    }
-
   if (argc == 0)
     grub_ls_list_disks (state[0].set);
   else

fs/fat.c

@@ -628,7 +628,9 @@ grub_fat_dir (grub_device_t device, const char *path,
 {
   struct grub_fat_data *data = 0;
   grub_disk_t disk = device->disk;
-  char *p = (char *) path;
+  grub_size_t len;
+  char *dirname;
+  char *p;
 
 #ifndef GRUB_UTIL
   grub_dl_ref (my_mod);
@@ -638,6 +640,18 @@ grub_fat_dir (grub_device_t device, const char *path,
   if (! data)
     goto fail;
 
+  /* Make sure that DIRNAME terminates with '/'.  */
+  len = grub_strlen (path);
+  dirname = grub_malloc (len + 1 + 1);
+  if (! dirname)
+    goto fail;
+  grub_memcpy (dirname, path, len);
+  p = dirname + len;
+  if (path[len - 1] != '/')
+    *p++ = '/';
+  *p = '\0';
+  p = dirname;
+  
   do
     {
       p = grub_fat_find_dir (disk, data, p, hook);
@@ -646,6 +660,7 @@ grub_fat_dir (grub_device_t device, const char *path,
 
  fail:
 
+  grub_free (dirname);
   grub_free (data);
   
 #ifndef GRUB_UTIL

include/grub/device.h

@@ -1,7 +1,7 @@
 /* device.h - device manager */
 /*
  *  GRUB  --  GRand Unified Bootloader
- *  Copyright (C) 2002  Free Software Foundation, Inc.
+ *  Copyright (C) 2002,2005  Free Software Foundation, Inc.
  *
  *  GRUB is free software; you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License as published by
@@ -38,7 +38,4 @@ typedef struct grub_device *grub_device_t;
 grub_device_t EXPORT_FUNC(grub_device_open) (const char *name);
 grub_err_t EXPORT_FUNC(grub_device_close) (grub_device_t device);
 
-grub_err_t EXPORT_FUNC(grub_device_set_root) (const char *name);
-const char *EXPORT_FUNC(grub_device_get_root) (void);
-
 #endif /* ! GRUB_DEVICE_HEADER */

include/grub/env.h

@@ -1,6 +1,6 @@
 /*
  *  GRUB  --  GRand Unified Bootloader
- *  Copyright (C) 2003  Free Software Foundation, Inc.
+ *  Copyright (C) 2003,2005  Free Software Foundation, Inc.
  *
  *  GRUB is free software; you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License as published by
@@ -24,12 +24,19 @@
 #include <grub/err.h>
 #include <grub/types.h>
 
+struct grub_env_var;
+
+typedef char *(*grub_env_read_hook_t) (struct grub_env_var *var,
+				       const char *val);
+typedef char *(*grub_env_write_hook_t) (struct grub_env_var *var,
+					const char *val);
+
 struct grub_env_var
 {
   char *name;
   char *value;
-  grub_err_t (*read_hook) (struct grub_env_var *var, char **val);
+  grub_env_read_hook_t read_hook;
-  grub_err_t (*write_hook) (struct grub_env_var *var);
+  grub_env_write_hook_t write_hook;
   struct grub_env_var *next;
   struct grub_env_var **prevp;
   struct grub_env_var *sort_next;
@@ -41,9 +48,7 @@ char *EXPORT_FUNC(grub_env_get) (const char *name);
 void EXPORT_FUNC(grub_env_unset) (const char *name);
 void EXPORT_FUNC(grub_env_iterate) (int (* func) (struct grub_env_var *var));
 grub_err_t EXPORT_FUNC(grub_register_variable_hook) (const char *var,
-						     grub_err_t (*read_hook) 
+						     grub_env_read_hook_t read_hook,
-						     (struct grub_env_var *var, char **val),
+						     grub_env_write_hook_t write_hook);
-						     grub_err_t (*write_hook)
-						     (struct grub_env_var *var));
 
 #endif /* ! GRUB_ENV_HEADER */

kern/device.c

@@ -1,7 +1,7 @@
 /* device.c - device manager */
 /*
  *  GRUB  --  GRand Unified Bootloader
- *  Copyright (C) 2002  Free Software Foundation, Inc.
+ *  Copyright (C) 2002,2005  Free Software Foundation, Inc.
  *
  *  GRUB is free software; you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License as published by
@@ -24,25 +24,7 @@
 #include <grub/fs.h>
 #include <grub/mm.h>
 #include <grub/misc.h>
-
+#include <grub/env.h>
-static char *grub_device_root;
-
-grub_err_t
-grub_device_set_root (const char *name)
-{
-  grub_free (grub_device_root);
-  grub_device_root = grub_strdup (name);
-  return grub_errno;
-}
-
-const char *
-grub_device_get_root (void)
-{
-  if (! grub_device_root)
-    grub_error (GRUB_ERR_BAD_DEVICE, "no root device");
-  
-  return grub_device_root;
-}
 
 grub_device_t
 grub_device_open (const char *name)
@@ -52,13 +34,12 @@ grub_device_open (const char *name)
 
   if (! name)
     {
-      if (! grub_device_root)
+      name = grub_env_get ("root");
+      if (*name == '\0')
 	{
 	  grub_error (GRUB_ERR_BAD_DEVICE, "no device is set");
 	  goto fail;
 	}
-
-      name = grub_device_root;
     }
     
   dev = grub_malloc (sizeof (*dev));

kern/env.c

@@ -1,7 +1,7 @@
 /* env.c - Environment variables */
 /*
  *  GRUB  --  GRand Unified Bootloader
- *  Copyright (C) 2003  Free Software Foundation, Inc.
+ *  Copyright (C) 2003,2005  Free Software Foundation, Inc.
  *
  *  GRUB is free software; you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License as published by
@@ -22,8 +22,8 @@
 #include <grub/misc.h>
 #include <grub/mm.h>
 
-/* XXX: What would be a good size for the hashtable?  */
+/* The size of the hash table.  */
-#define	HASHSZ	123
+#define	HASHSZ	13
 
 /* A hashtable for quick lookup of variables.  */
 static struct grub_env_var *grub_env[HASHSZ];
@@ -68,16 +68,18 @@ grub_env_set (const char *var, const char *val)
   if (env)
     {
       char *old = env->value;
+
+      if (env->write_hook)
+	env->value = env->write_hook (env, val);
+      else
 	env->value = grub_strdup (val);
-      if (! env->name)
+      
+      if (! env->value)
 	{
 	  env->value = old;
 	  return grub_errno;
 	}
 
-      if (env->write_hook)
-	(env->write_hook) (env);
-
       grub_free (old);
       return 0;
     }
@@ -141,12 +143,7 @@ grub_env_get (const char *name)
     return 0;
 
   if (env->read_hook)
-    {
+    return env->read_hook (env, env->value);
-      char *val;
-      env->read_hook (env, &val);
-
-      return val;
-    }
 
   return env->value;
 }
@@ -190,15 +187,22 @@ grub_env_iterate (int (* func) (struct grub_env_var *var))
 
 grub_err_t
 grub_register_variable_hook (const char *var,
-			     grub_err_t (*read_hook) (struct grub_env_var *var, char **),
+			     grub_env_read_hook_t read_hook,
-			     grub_err_t (*write_hook) (struct grub_env_var *var))
+			     grub_env_write_hook_t write_hook)
 {
   struct grub_env_var *env = grub_env_find (var);
 
   if (! env)
-    if (grub_env_set (var, "") != GRUB_ERR_NONE)
+    {
+      char *val = grub_strdup ("");
+
+      if (! val)
 	return grub_errno;
       
+      if (grub_env_set (var, val) != GRUB_ERR_NONE)
+	return grub_errno;
+    }
+  
   env = grub_env_find (var);
   /* XXX Insert an assertion?  */
   

kern/main.c

@@ -57,12 +57,28 @@ grub_load_modules (void)
   grub_mm_init_region ((void *) modinfo, modinfo->size);
 }
 
+/* Write hook for the environment variables of root. Remove surrounding
+   parentheses, if any.  */
+static char *
+grub_env_write_root (struct grub_env_var *var, const char *val)
+{
+  /* XXX Is it better to check the existence of the device?  */
+  grub_size_t len = grub_strlen (val);
+  
+  if (val[0] == '(' && val[len - 1] == ')')
+    return grub_strndup (val + 1, len - 2);
+
+  return grub_strdup (val);
+}
+
 /* Set the root device according to the dl prefix.  */
 static void
 grub_set_root_dev (void)
 {
   const char *prefix;
 
+  grub_register_variable_hook ("root", 0, grub_env_write_root);
+  
   prefix = grub_env_get ("prefix");
   
   if (prefix)
@@ -72,7 +88,7 @@ grub_set_root_dev (void)
       dev = grub_file_get_device_name (prefix);
       if (dev)
 	{
-	  grub_device_set_root (dev);
+	  grub_env_set ("root", dev);
 	  grub_free (dev);
 	}
     }

kern/rescue.c

@@ -337,7 +337,7 @@ grub_rescue_cmd_root (int argc, char *argv[])
       if (! device_name)
 	return;
 
-      grub_device_set_root (device_name);
+      grub_env_set ("root", device_name);
       grub_free (device_name);
     }
   
@@ -350,7 +350,7 @@ grub_rescue_cmd_root (int argc, char *argv[])
     grub_errno = GRUB_ERR_NONE;
   
   grub_printf ("(%s): Filesystem is %s.\n",
-	       grub_device_get_root (), fs ? fs->name : "unknown");
+	       grub_env_get ("root"), fs ? fs->name : "unknown");
   
   grub_device_close (dev);
 }

util/i386/pc/grub-setup.c

@@ -27,6 +27,7 @@
 #include <grub/fs.h>
 #include <grub/partition.h>
 #include <grub/pc_partition.h>
+#include <grub/env.h>
 #include <grub/machine/util/biosdisk.h>
 #include <grub/machine/boot.h>
 #include <grub/machine/kernel.h>
@@ -210,7 +211,7 @@ setup (const char *prefix, const char *dir,
     grub_util_error ("%s", grub_errmsg);
 
   grub_util_info ("setting the root device to `%s'", root);
-  if (grub_device_set_root (root) != GRUB_ERR_NONE)
+  if (grub_env_set ("root", root) != GRUB_ERR_NONE)
     grub_util_error ("%s", grub_errmsg);
 
   /* Read the original sector from the disk.  */