Automagic command loading

Automagic command loading

[Tomas Ebenlendr]

Here is new patch. The commands list is stored in elf section
.uinfo.norm_cmds. The module searching mechanism is in autocmd.mod,
so you switch on autoloading by inserting this module. Interface
is designed such that there may be other modules autoregistering
commands.

The contents of .uinfo.norm_cmds of all modules in $prefix gets
cached on insert of autocmd.mod, because reading of all modules takes
about 5 seconds on my bochs under Linux, 300Mhz Pentium. The contents
is recached by command cache_autocommands or when $prefix does not
match cached directory. This will be used on very rare ocasions.

Please feel free to comment anything. I will commit this patch if there
will be no comments (and no my changes) in two weeks.

P.S.: There is no common code with the old patch. The old was only to
see how it could (or couldn't) be done.
======================================================
Changelog:
/DATE/  Tomas Ebenlendr  <ebik@ucw.cz>

	Added support for normal mode commands autoloading.

	conf/i386-pc.rmk (pkgdata_MODULES): New module autocmd.mod
	(autocmd_mod_SOURCES, autocmd_mod_ASFLAGS): Likewise.

	commands/boot.c (GRUB_MOD_INIT): NORMAL_COMMAND() marking of command.
	This ensures thas command name occur in special elf section.
	commands/cat.c: Likewise.
	commands/cmp.c: Likewise.
	commands/ls.c: Likewise.
	commands/terminal.c: Likewise.
	font/manager.c: Likewise.
	hello/hello.c: Likewise.
	loader/i386/pc/chainloader_normal.c: Likewise.
	loader/i386/pc/linux_normal.c: Likewise.
	loader/i386/pc/multiboot_normal.c: Likewise.
	loader/powerpc/ieee1275/linux_normal.c: Likewise.
	
	gencmdlist.sh: New file. Greps NORMAL_COMMAND in sources and generates
	list of commands (c source).

	genmk.rb (PModule:rule): Added targets #{cmds_src} and #{cmds_obj}, of
	normal mode commands list.

	include/grub/dl.h (GRUB_MOD_UINFO): New macro. Adds string to user
	section in elf format.
	(grub_dl_read_file_uinfo): Reads that string.

	kern/dl.c (grub_dl_read_file_uinfo): New function.

	incude/grub/normal.h (NORMAL_COMMAND): New macro. Used for marking
	commands to appear in special elf section.
	(grub_command_autoloader_t, struct grub_command_autoloader): New type,
	list of autoloaders.
	(grub_register_command_autoloader): New exported prototype.
	(grub_unregister_command_autoloader): New exported prototype.
	normal/command.c (grub_register_command_autoloader): New function.
	(grub_unregister_command_autoloader): New function.
	(grub_command_find): Execute autoloaders when command not found.
	

	normal/autocmd.c: New file. Implements searching of module.

	kern/misc.c (grub_strndup): Copy only len characters and add ending NUL.

diff -rupN -x CVS -x '*.mk' grub2_savannah/commands/boot.c grub2_autocmd/commands/boot.c
--- grub2_savannah/commands/boot.c	2004-08-24 23:38:49.000000000 +0200
+++ grub2_autocmd/commands/boot.c	2004-09-26 18:17:53.000000000 +0200
@@ -55,7 +55,7 @@ grub_boot_fini (void)
 GRUB_MOD_INIT
 {
   (void)mod;			/* To stop warning. */
-  grub_register_command ("boot", grub_cmd_boot, GRUB_COMMAND_FLAG_BOTH,
+  grub_register_command (NORMAL_COMMAND("boot"), grub_cmd_boot, GRUB_COMMAND_FLAG_BOTH,
 			 "boot", "Boot an operating system", 0);
 }
 
diff -rupN -x CVS -x '*.mk' grub2_savannah/commands/cat.c grub2_autocmd/commands/cat.c
--- grub2_savannah/commands/cat.c	2004-04-04 15:46:00.000000000 +0200
+++ grub2_autocmd/commands/cat.c	2004-09-28 19:43:05.000000000 +0200
@@ -86,7 +86,7 @@ grub_cat_fini (void)
 GRUB_MOD_INIT
 {
   (void)mod;			/* To stop warning. */
-  grub_register_command ("cat", grub_cmd_cat, GRUB_COMMAND_FLAG_BOTH,
+  grub_register_command (NORMAL_COMMAND("cat"), grub_cmd_cat, GRUB_COMMAND_FLAG_BOTH,
 			 "cat FILE", "Show the contents of a file", 0);
 }
 
diff -rupN -x CVS -x '*.mk' grub2_savannah/commands/cmp.c grub2_autocmd/commands/cmp.c
--- grub2_savannah/commands/cmp.c	2004-04-04 15:46:00.000000000 +0200
+++ grub2_autocmd/commands/cmp.c	2004-09-28 19:43:30.000000000 +0200
@@ -113,7 +113,7 @@ grub_cmp_fini (void)
 GRUB_MOD_INIT
 {
   (void)mod;			/* To stop warning. */
-  grub_register_command ("cmp", grub_cmd_cmp, GRUB_COMMAND_FLAG_BOTH,
+  grub_register_command (NORMAL_COMMAND("cmp"), grub_cmd_cmp, GRUB_COMMAND_FLAG_BOTH,
 			 "cmp FILE1 FILE2", "Compare two files", 0);
 }
 
diff -rupN -x CVS -x '*.mk' grub2_savannah/commands/ls.c grub2_autocmd/commands/ls.c
--- grub2_savannah/commands/ls.c	2004-04-04 15:46:00.000000000 +0200
+++ grub2_autocmd/commands/ls.c	2004-09-28 19:43:58.000000000 +0200
@@ -113,7 +113,7 @@ grub_ls_list_files (const char *dirname,
      
   static int print_files_long (const char *filename, int dir)
     {
-      char pathname[grub_strlen (dirname) + grub_strlen (filename) + 1];
+      char pathname[grub_strlen (dirname) + grub_strlen (filename) + 2];
 
       if ((! all) && (filename[0] == '.'))
 	return 0;
@@ -250,7 +250,7 @@ grub_ls_fini (void)
 GRUB_MOD_INIT
 {
   (void)mod;			/* To stop warning. */
-  grub_register_command ("ls", grub_cmd_ls, GRUB_COMMAND_FLAG_BOTH,
+  grub_register_command (NORMAL_COMMAND("ls"), grub_cmd_ls, GRUB_COMMAND_FLAG_BOTH,
 			 "ls [OPTIONS...] [DIR]",
 			 "List devices and files", options);
 }
diff -rupN -x CVS -x '*.mk' grub2_savannah/commands/terminal.c grub2_autocmd/commands/terminal.c
--- grub2_savannah/commands/terminal.c	2004-04-04 15:46:00.000000000 +0200
+++ grub2_autocmd/commands/terminal.c	2004-09-28 19:44:23.000000000 +0200
@@ -88,7 +88,7 @@ grub_terminal_fini (void)
 GRUB_MOD_INIT
 {
   (void)mod;			/* To stop warning. */
-  grub_register_command ("terminal", grub_cmd_terminal, GRUB_COMMAND_FLAG_BOTH,
+  grub_register_command (NORMAL_COMMAND("terminal"), grub_cmd_terminal, GRUB_COMMAND_FLAG_BOTH,
 			 "terminal [TERM...]", "Select a terminal.", 0);
 }
 
diff -rupN -x CVS -x '*.mk' grub2_savannah/conf/i386-pc.rmk grub2_autocmd/conf/i386-pc.rmk
--- grub2_savannah/conf/i386-pc.rmk	2004-09-20 09:23:23.000000000 +0200
+++ grub2_autocmd/conf/i386-pc.rmk	2004-09-27 21:29:55.000000000 +0200
@@ -80,7 +80,8 @@ genmoddep_SOURCES = util/genmoddep.c
 # Modules.
 pkgdata_MODULES = _chain.mod _linux.mod linux.mod fat.mod ufs.mod ext2.mod minix.mod \
 	hfs.mod jfs.mod normal.mod hello.mod vga.mod font.mod _multiboot.mod ls.mod \
-	boot.mod cmp.mod cat.mod terminal.mod fshelp.mod chain.mod multiboot.mod
+	boot.mod cmp.mod cat.mod terminal.mod fshelp.mod chain.mod multiboot.mod \
+	autocmd.mod
 
 # For _chain.mod.
 _chain_mod_SOURCES = loader/i386/pc/chainloader.c
@@ -132,6 +133,10 @@ normal_mod_SOURCES = normal/cmdline.c no
 normal_mod_CFLAGS = $(COMMON_CFLAGS)
 normal_mod_ASFLAGS = $(COMMON_ASFLAGS)
 
+# For autocmd.mod.
+autocmd_mod_SOURCES = normal/autocmd.c
+autocmd_mod_CFLAGS = $(COMMON_CFLAGS)
+
 # For hello.mod.
 hello_mod_SOURCES = hello/hello.c
 hello_mod_CFLAGS = $(COMMON_CFLAGS)
diff -rupN -x CVS -x '*.mk' grub2_savannah/font/manager.c grub2_autocmd/font/manager.c
--- grub2_savannah/font/manager.c	2004-04-04 15:46:00.000000000 +0200
+++ grub2_autocmd/font/manager.c	2004-09-28 19:44:57.000000000 +0200
@@ -235,7 +235,7 @@ font_command (struct grub_arg_list *stat
 GRUB_MOD_INIT
 {
   (void) mod; /* Stop warning.  */
-  grub_register_command ("font", font_command, GRUB_COMMAND_FLAG_BOTH,
+  grub_register_command (NORMAL_COMMAND("font"), font_command, GRUB_COMMAND_FLAG_BOTH,
 			 "font FILE...", "Specify a font file to display.", 0);
 }
 
diff -rupN -x CVS -x '*.mk' grub2_savannah/gencmdlist.sh grub2_autocmd/gencmdlist.sh
--- grub2_savannah/gencmdlist.sh	1970-01-01 01:00:00.000000000 +0100
+++ grub2_autocmd/gencmdlist.sh	2004-09-27 15:09:38.000000000 +0200
@@ -0,0 +1,38 @@
+#! /bin/sh
+#
+# Copyright (C) 2004  Free Software Foundation, Inc.
+#
+# This gensymlist.sh is free software; the author
+# gives unlimited permission to copy and/or distribute it,
+# with or without modifications, as long as this notice is preserved.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY, to the extent permitted by law; without
+# even the implied warranty of MERCHANTABILITY or FITNESS FOR A
+# PARTICULAR PURPOSE.
+
+cat <<EOF
+/* This file is automatically generated by gencmdlist.sh. DO NOT EDIT! */
+/*
+ *  GRUB  --  GRand Unified Bootloader
+ *  Copyright (C) 2004  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
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with GRUB; if not, write to the Free Software
+ *  Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+
+#include <grub/dl.h>
+EOF
+
+cat $* | grep -v '^#' | sed -n 's/.*NORMAL_COMMAND *( *\" *\([a-zA-Z0-9_]*\) *\" *).*/GRUB_MOD_UINFO("norm_cmds","\1")\;/;T;p;'
diff -rupN -x CVS -x '*.mk' grub2_savannah/genmk.rb grub2_autocmd/genmk.rb
--- grub2_savannah/genmk.rb	2004-04-04 15:45:59.000000000 +0200
+++ grub2_autocmd/genmk.rb	2004-09-27 14:56:14.000000000 +0200
@@ -109,13 +109,15 @@ class PModule
     defsym = 'def-' + @name.suffix('lst')
     undsym = 'und-' + @name.suffix('lst')
     mod_name = File.basename(@name, '.mod')
+    cmds_src = 'cmd-' + @name.suffix('c')
+    cmds_obj = 'cmd-' + @name.suffix('o')
     
-    "CLEANFILES += #{@name} #{mod_obj} #{mod_src} #{pre_obj} #{objs_str} #{defsym} #{undsym}
+    "CLEANFILES += #{@name} #{mod_obj} #{mod_src} #{pre_obj} #{objs_str} #{defsym} #{undsym} #{cmds_src} #{cmds_obj}
 MOSTLYCLEANFILES += #{deps_str}
 DEFSYMFILES += #{defsym}
 UNDSYMFILES += #{undsym}
 
-#{@name}: #{pre_obj} #{mod_obj}
+#{@name}: #{pre_obj} #{mod_obj} #{cmds_obj}
 	-rm -f $@
 	$(LD) -r -o $@ $^
 	$(STRIP) --strip-unneeded -K grub_mod_init -K grub_mod_fini -R .note -R .comment $@
@@ -127,7 +129,7 @@ UNDSYMFILES += #{undsym}
 #{mod_obj}: #{mod_src}
 	$(CC) $(CPPFLAGS) $(CFLAGS) $(#{prefix}_CFLAGS) -c -o $@ $<
 
-#{mod_src}: moddep.lst genmodsrc.sh
+#{mod_src}: moddep.lst $(srcdir)/genmodsrc.sh
 	sh $(srcdir)/genmodsrc.sh '#{mod_name}' $< > $@ || (rm -f $@; exit 1)
 
 #{defsym}: #{pre_obj}
@@ -137,6 +139,12 @@ UNDSYMFILES += #{undsym}
 	echo '#{mod_name}' > $@
 	$(NM) -u -P -p $< | cut -f1 -d' ' >> $@
 
+#{cmds_obj}: #{cmds_src}
+	$(CC) $(CPPFLAGS) $(CFLAGS) $(#{prefix}_CFLAGS) -c -o $@ $<
+
+#{cmds_src}: $(srcdir)/gencmdlist.sh $(#{prefix}_SOURCES)
+	sh $^ >$@ || (rm -f $@; exit 1)
+
 " + objs.collect_with_index do |obj, i|
       src = sources[i]
       fake_obj = File.basename(src).suffix('o')
diff -rupN -x CVS -x '*.mk' grub2_savannah/hello/hello.c grub2_autocmd/hello/hello.c
--- grub2_savannah/hello/hello.c	2004-04-04 15:46:00.000000000 +0200
+++ grub2_autocmd/hello/hello.c	2004-09-28 19:45:13.000000000 +0200
@@ -38,7 +38,7 @@ grub_cmd_hello (struct grub_arg_list *st
 GRUB_MOD_INIT
 {
   (void)mod;			/* To stop warning. */
-  grub_register_command ("hello", grub_cmd_hello, GRUB_COMMAND_FLAG_BOTH,
+  grub_register_command (NORMAL_COMMAND("hello"), grub_cmd_hello, GRUB_COMMAND_FLAG_BOTH,
 			 "hello", "Say hello", 0);
 }
 
diff -rupN -x CVS -x '*.mk' grub2_savannah/include/grub/dl.h grub2_autocmd/include/grub/dl.h
--- grub2_savannah/include/grub/dl.h	2004-04-04 15:46:00.000000000 +0200
+++ grub2_autocmd/include/grub/dl.h	2004-09-28 19:58:43.000000000 +0200
@@ -41,6 +41,10 @@ __asm__ (".section .modname,\"S\"\n.stri
 #define GRUB_MOD_DEP(name)	\
 __asm__ (".section .moddeps,\"S\"\n.string \"" #name "\"\n.previous")
 
+/* Any user information in module (string).  */
+#define GRUB_MOD_UINFO(section, value)     \
+__asm__ (".section .uinfo." section ",\"S\"\n.string \"" value "\"\n.previous")
+
 struct grub_dl_segment
 {
   struct grub_dl_segment *next;
@@ -83,6 +87,7 @@ grub_dl_t EXPORT_FUNC(grub_dl_get) (cons
 grub_err_t EXPORT_FUNC(grub_dl_register_symbol) (const char *name, void *addr,
 					    grub_dl_t mod);
 void *EXPORT_FUNC(grub_dl_resolve_symbol) (const char *name);
+grub_err_t EXPORT_FUNC(grub_dl_read_file_uinfo) (const char *filename, const char * secname, char ** value, int * len);
 
 int grub_arch_dl_check_header (void *ehdr, grub_size_t size);
 grub_err_t grub_arch_dl_relocate_symbols (grub_dl_t mod, void *ehdr);
diff -rupN -x CVS -x '*.mk' grub2_savannah/include/grub/normal.h grub2_autocmd/include/grub/normal.h
--- grub2_savannah/include/grub/normal.h	2004-09-20 09:23:31.000000000 +0200
+++ grub2_autocmd/include/grub/normal.h	2004-09-27 21:34:29.000000000 +0200
@@ -25,6 +25,7 @@
 #include <grub/symbol.h>
 #include <grub/err.h>
 #include <grub/arg.h>
+#include <grub/dl.h>
 
 /* The maximum size of a command-line.  */
 #define GRUB_MAX_CMDLINE	1600
@@ -42,6 +43,8 @@
 /* Don't print the command on booting.  */
 #define GRUB_COMMAND_FLAG_NO_ARG_PARSE	0x10
 
+#define NORMAL_COMMAND(x) x
+
 /* The command description.  */
 struct grub_command
 {
@@ -68,6 +71,17 @@ struct grub_command
 };
 typedef struct grub_command *grub_command_t;
 
+/* The command autoloader description.  */
+struct grub_command_autoloader
+{
+  /* The callback function.  */
+  grub_dl_t (*func) (const char * command);
+
+  /* The next element.  */
+  struct grub_command_autoloader *next;
+};
+typedef struct grub_command_autoloader *grub_command_autoloader_t;
+
 /* The command list.  */
 struct grub_command_list
 {
@@ -141,6 +155,10 @@ void grub_command_init (void);
 void grub_normal_init_page (void);
 int grub_arg_parse (grub_command_t parser, int argc, char **argv,
 		    struct grub_arg_list *usr, char ***args, int *argnum);
+void EXPORT_FUNC(grub_register_command_autoloader) (
+			    grub_dl_t (*hook) (const char * cmd));
+void EXPORT_FUNC(grub_unregister_command_autoloader) (
+			    grub_dl_t (*hook) (const char * cmd));
 
 
 #ifdef GRUB_UTIL
diff -rupN -x CVS -x '*.mk' grub2_savannah/kern/dl.c grub2_autocmd/kern/dl.c
--- grub2_savannah/kern/dl.c	2004-08-12 23:39:00.000000000 +0200
+++ grub2_autocmd/kern/dl.c	2004-09-28 19:59:33.000000000 +0200
@@ -590,6 +590,97 @@ grub_dl_load (const char *name)
   return mod;
 }
 
+#define MEMFAILED \
+  ({\
+    grub_error (GRUB_ERR_OUT_OF_MEMORY, "Out of memory");\
+    goto failed;\
+  })
+
+#define READFAILED \
+  ({\
+    grub_error (GRUB_ERR_FILE_READ_ERROR, "Error reading file");\
+    goto failed;\
+  })
+    
+/* Reads value of user information in file (elf - module).
+   Don't forget to free() result.  */
+grub_err_t
+grub_dl_read_file_uinfo (const char *filename, const char * secname, char ** value, int * len)
+{
+  Elf_Ehdr e;
+  Elf_Shdr *s = 0;
+  Elf_Shdr *ps;
+  char *str = 0;
+  char *section = 0;
+  unsigned i;
+  grub_file_t file = 0;
+  int readbytes;
+  int wantbytes;
+  grub_err_t ret = -1;
+
+  section = grub_malloc (grub_strlen (secname) + 8);
+  if (! section)
+    goto failed;
+  grub_strcpy (section, ".uinfo.");
+  grub_strcpy (section + 7, secname);
+
+  file = grub_file_open (filename);
+  if (! file)
+    goto failed;
+  
+  readbytes = grub_file_read (file,(char *) &e, sizeof (e));
+  if ((readbytes < (signed) sizeof (e)) || (e.e_ehsize < readbytes) || (e.e_shentsize < (signed) sizeof (*s)))
+    READFAILED;
+
+  wantbytes = e.e_shnum * e.e_shentsize;
+  s = grub_malloc (wantbytes);
+  if (! s)
+    MEMFAILED;
+  grub_file_seek (file, e.e_shoff);
+  readbytes = grub_file_read (file,(char *) s, wantbytes);
+  if (wantbytes != readbytes)
+    READFAILED;
+
+  ps = (Elf_Shdr *) ((char *) s + e.e_shstrndx * e.e_shentsize);
+  wantbytes = ps->sh_size;
+  str = grub_malloc (wantbytes);
+  if (! str)
+    MEMFAILED;
+  grub_file_seek (file, ps->sh_offset);
+  readbytes = grub_file_read (file, str, wantbytes);
+  if (wantbytes != readbytes)
+    READFAILED;
+  
+  for (i = 0, ps = s;
+       i < e.e_shnum;
+       i++, ps = (Elf_Shdr *) ((char *) ps + e.e_shentsize))
+    if (grub_strcmp (str + ps->sh_name, section) == 0)
+      {
+	wantbytes = *len = ps->sh_size;
+	*value = grub_malloc (wantbytes);
+	if (! *value)
+	  MEMFAILED;
+	grub_file_seek (file, ps->sh_offset);
+	readbytes = grub_file_read (file, *value, wantbytes);
+	if (wantbytes != readbytes)
+	  READFAILED;
+	
+	ret = 0;
+	break;
+      }
+
+  if (0)
+    {
+failed:
+      ret = grub_errno;
+    }
+  grub_free (str);
+  grub_free (s);
+  grub_file_close (file);
+  grub_free (section);
+  return ret;
+}
+
 /* Unload the module MOD.  */
 int
 grub_dl_unload (grub_dl_t mod)
diff -rupN -x CVS -x '*.mk' grub2_savannah/kern/misc.c grub2_autocmd/kern/misc.c
--- grub2_savannah/kern/misc.c	2004-09-20 09:23:31.000000000 +0200
+++ grub2_autocmd/kern/misc.c	2004-09-28 02:07:13.000000000 +0200
@@ -342,15 +342,17 @@ grub_strndup (const char *s, grub_size_t
   grub_size_t len = 0;
   char *p = (char *) s;
   
-  while (*(p++) && len < n)
-    len++;
+  while (*p && n > (unsigned) (p - s))
+    p++;
+  len = p - s;
 
-  len = grub_strlen (s) + 1;
-  p = (char *) grub_malloc (len);
+  p = (char *) grub_malloc (len + 1);
   if (! p)
     return 0;
 
-  return grub_memcpy (p, s, len);
+  p = grub_memcpy (p, s, len);
+  p[len] = 0;
+  return p;
 }
 
 void *
diff -rupN -x CVS -x '*.mk' grub2_savannah/loader/i386/pc/chainloader_normal.c grub2_autocmd/loader/i386/pc/chainloader_normal.c
--- grub2_savannah/loader/i386/pc/chainloader_normal.c	2004-09-12 14:20:52.000000000 +0200
+++ grub2_autocmd/loader/i386/pc/chainloader_normal.c	2004-09-28 19:46:17.000000000 +0200
@@ -45,7 +45,7 @@ chainloader_command (struct grub_arg_lis
 GRUB_MOD_INIT
 {
   (void) mod; /* To stop warning.  */
-  grub_register_command ("chainloader", chainloader_command,
+  grub_register_command (NORMAL_COMMAND("chainloader"), chainloader_command,
 			 GRUB_COMMAND_FLAG_BOTH,
 			 "chainloader [options] FILE",
 			 "Prepare to boot another boot loader", options);
diff -rupN -x CVS -x '*.mk' grub2_savannah/loader/i386/pc/linux_normal.c grub2_autocmd/loader/i386/pc/linux_normal.c
--- grub2_savannah/loader/i386/pc/linux_normal.c	2004-09-17 11:36:52.000000000 +0200
+++ grub2_autocmd/loader/i386/pc/linux_normal.c	2004-09-28 19:46:44.000000000 +0200
@@ -43,12 +43,12 @@ grub_normal_initrd_command (struct grub_
 GRUB_MOD_INIT
 {
   (void) mod; /* To stop warning.  */
-  grub_register_command ("linux", grub_normal_linux_command,
+  grub_register_command (NORMAL_COMMAND("linux"), grub_normal_linux_command,
 			 GRUB_COMMAND_FLAG_BOTH,
 			 "linux FILE [ARGS...]",
 			 "Load linux", 0);
   
-  grub_register_command ("initrd", grub_normal_initrd_command,
+  grub_register_command (NORMAL_COMMAND("initrd"), grub_normal_initrd_command,
 			 GRUB_COMMAND_FLAG_BOTH,
 			 "initrd FILE",
 			 "Load initrd", 0);
diff -rupN -x CVS -x '*.mk' grub2_savannah/loader/i386/pc/multiboot_normal.c grub2_autocmd/loader/i386/pc/multiboot_normal.c
--- grub2_savannah/loader/i386/pc/multiboot_normal.c	2004-09-17 11:36:52.000000000 +0200
+++ grub2_autocmd/loader/i386/pc/multiboot_normal.c	2004-09-28 19:47:14.000000000 +0200
@@ -43,12 +43,12 @@ grub_normal_cmd_module (struct grub_arg_
 GRUB_MOD_INIT
 {
   (void) mod; /* To stop warning.  */
-  grub_register_command ("multiboot", grub_normal_cmd_multiboot,
+  grub_register_command (NORMAL_COMMAND("multiboot"), grub_normal_cmd_multiboot,
 			 GRUB_COMMAND_FLAG_BOTH | GRUB_COMMAND_FLAG_NO_ARG_PARSE,
 			 "multiboot FILE [ARGS...]",
 			 "Load a multiboot kernel", 0);
   
-  grub_register_command ("module", grub_normal_cmd_module,
+  grub_register_command (NORMAL_COMMAND("module"), grub_normal_cmd_module,
 			 GRUB_COMMAND_FLAG_BOTH | GRUB_COMMAND_FLAG_NO_ARG_PARSE,
 			 "multiboot FILE [ARGS...]",
 			 "Load a multiboot module", 0);
diff -rupN -x CVS -x '*.mk' grub2_savannah/loader/powerpc/ieee1275/linux_normal.c grub2_autocmd/loader/powerpc/ieee1275/linux_normal.c
--- grub2_savannah/loader/powerpc/ieee1275/linux_normal.c	2004-07-27 19:47:37.000000000 +0200
+++ grub2_autocmd/loader/powerpc/ieee1275/linux_normal.c	2004-09-28 19:47:36.000000000 +0200
@@ -38,7 +38,7 @@ grub_cmd_linux (struct grub_arg_list *st
 GRUB_MOD_INIT
 {
   (void) mod;
-  grub_register_command ("linux", grub_cmd_linux, GRUB_COMMAND_FLAG_BOTH,
+  grub_register_command (NORMAL_COMMAND("linux"), grub_cmd_linux, GRUB_COMMAND_FLAG_BOTH,
 			 "linux [KERNELARGS...]",
 			 "Loads linux", options);
 }
diff -rupN -x CVS -x '*.mk' grub2_savannah/normal/autocmd.c grub2_autocmd/normal/autocmd.c
--- grub2_savannah/normal/autocmd.c	1970-01-01 01:00:00.000000000 +0100
+++ grub2_autocmd/normal/autocmd.c	2004-09-28 20:05:25.000000000 +0200
@@ -0,0 +1,267 @@
+/*
+ *  GRUB  --  GRand Unified Bootloader
+ *  Copyright (C) 2004  Free Software Foundation, Inc.
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+
+#include <grub/err.h>
+#include <grub/dl.h>
+#include <grub/env.h>
+#include <grub/misc.h>
+#include <grub/fs.h>
+#include <grub/file.h>
+#include <grub/mm.h>
+#include <grub/normal.h>
+
+struct module_commands
+{
+  char *module;
+  char *commands;
+  int commands_len;
+  struct module_commands *next;
+};
+typedef struct module_commands *module_commands_t;
+
+char * dirname = 0;
+module_commands_t modules_list = 0;
+
+static void
+clear_commands_list (void)
+{
+  module_commands_t p, q;
+
+  for (p = modules_list; p; p=q)
+    {
+      q = p->next;
+
+      grub_free (p->module);
+      grub_free (p->commands);
+      grub_free (p);
+    }
+  modules_list = 0;
+}
+
+static grub_err_t
+cache_uinfo_files (struct grub_arg_list *state __attribute__ ((unused)),
+	       int argc __attribute__ ((unused)),
+	       char **args __attribute__ ((unused)))
+{
+  int len;
+  char *path;
+  module_commands_t *p = &modules_list;
+  char *device_name = 0;
+  grub_device_t dev = 0;
+  grub_fs_t fs;
+
+  static int cache_module (const char *filename, int dir)
+    {
+      char * pathname = 0;
+      int suffixidx;
+      char *secbuf = 0;
+      int seclen;
+
+      /* Test that module is regular file, with .mod suffix.  */
+      if (dir)
+	return 0;
+      suffixidx = grub_strlen (filename) - 4;
+      if ((suffixidx <= 0) || (grub_strcmp (filename + suffixidx,".mod") != 0))
+	return 0;
+
+      /* Construct path.  */
+      pathname = grub_malloc (grub_strlen (dirname) + grub_strlen (filename) + 1);
+      if (! pathname)
+	return 0;
+      grub_sprintf (pathname, "%s%s", dirname, filename);
+
+      /* Test if module has in .uinfo.norm_cmds written the command name.  */
+      if (grub_dl_read_file_uinfo (pathname, "norm_cmds", &secbuf, &seclen))
+	goto ret;
+
+      /* Cache the uinfo.  */
+      *p = grub_malloc (sizeof (struct module_commands));
+      if (! (*p))
+	goto ret;
+
+      (*p)->module = grub_strdup (filename);
+      if (! (*p)->module)
+	{
+	  grub_free (*p);
+	  *p = 0;
+	  goto ret;
+	}
+
+      (*p)->commands = secbuf;
+      (*p)->commands_len = seclen;
+      p = &((*p)->next);
+      *p = 0;
+      if (0)
+	{
+ret:
+	  grub_free (secbuf);
+	}
+      grub_free (pathname);
+      return 0;
+    }
+  clear_commands_list ();
+  grub_free (dirname);
+
+  /* Create directory name.  */
+  path = grub_env_get ("prefix");
+  if (! path)
+    return 0;
+  len = grub_strlen (path);
+  if (len <= 0)
+    return 0;
+  dirname = grub_malloc (len + 2);
+  grub_memcpy (dirname, path, len);
+  if (dirname[len - 1] != '/')
+    dirname[len++] = '/';
+  dirname[len] = 0;
+
+  /* Read directory, cache_module() on each entry.  */
+  device_name = grub_file_get_device_name (dirname);
+  dev = grub_device_open (device_name);
+  if (! dev)
+    goto fail;
+
+  fs = grub_fs_probe (dev);
+  path = grub_strchr (dirname, ')') + 1;
+
+  if (! path || ! fs || (path[0] != '/'))
+      goto fail;
+      
+  (fs->dir) (dev, path, cache_module);
+
+fail:
+  if (dev)
+    grub_device_close (dev);
+      
+  grub_free (device_name);
+
+  return 0;
+}
+
+
+/* Load module containing command.  */
+static grub_dl_t
+grub_autocmd_load_module_by_cmd (const char *command)
+{
+  
+  char *modname;
+  char *path;
+  int len;
+  grub_dl_t mod = 0;
+  module_commands_t cmd;
+
+  static int try_module (module_commands_t p)
+    {
+      char *pathname = 0;
+      int suffixidx;
+      char *name;
+      char *s;
+
+      /* Test if module has in .uinfo.norm_cmds written the command name.  */
+      s = p->commands;
+      while (s - p->commands < p->commands_len)
+	{
+	  if (grub_strcmp (s, command) == 0)
+	    break;
+	  s += grub_strlen (s) + 1;
+	}
+      if (s >= p->commands + p->commands_len)
+	return 0;
+
+      /* Test that module is not loaded.  */
+      suffixidx = grub_strlen (p->module) - 4;
+      name = grub_strndup (p->module,suffixidx);
+      if (! name)
+	return 0;
+      if (grub_dl_get (name))
+	{
+	  grub_free (name);
+	  return 0;
+	}
+      grub_free (name);
+
+      /* Construct path.  */
+      pathname = grub_malloc (grub_strlen (dirname) + grub_strlen (p->module) + 1);
+      if (! pathname)
+	return 0;
+      grub_sprintf (pathname, "%s%s", dirname, p->module);
+
+      /* Load the module.  */
+      mod = grub_dl_load_file (pathname);
+      grub_free (pathname);
+
+      /* Let refcount be 0, module can get autoloaded again if unloaded.  */
+      if (mod)
+	return 1;
+
+      return 0;
+    }
+
+  /* Check if prefix is cached.  */
+  path = grub_env_get ("prefix");
+  if (! path)
+    return 0;
+  len = grub_strlen (path);
+
+  /* We may differ by ending '/'.  */
+  if ((grub_strncmp (path, dirname, len) != 0) ||
+      (dirname[len] && ((dirname[len] != '/') || dirname[len+1])))
+    cache_uinfo_files (0, 0, 0);
+
+  /* Try <commandname>.mod.  */
+  len = grub_strlen (command);
+  modname = grub_malloc (len + 5);
+  if (! modname)
+    goto fail;
+  grub_memcpy (modname, command, len);
+  grub_memcpy (modname + len, ".mod", 5);
+
+  for (cmd = modules_list; cmd; cmd = cmd->next)
+    if (grub_strcmp (modname, cmd->module) == 0)
+      break;
+
+  /* If successfull goto end.  */
+  if (cmd && (try_module (cmd)))
+    goto fail;
+
+  /* Try all modules.  */
+  for (cmd = modules_list; cmd; cmd = cmd->next)
+    if (try_module (cmd))
+      break;
+
+fail:
+  return mod;
+}
+
+GRUB_MOD_INIT
+{
+  (void)mod;			/* To stop warning. */
+  grub_register_command_autoloader (grub_autocmd_load_module_by_cmd);
+  grub_register_command ("cache_autocommands", cache_uinfo_files, GRUB_COMMAND_FLAG_BOTH,
+			 "cache_autocommands", "Recache information about commands in modules.", 0);
+  cache_uinfo_files (0, 0, 0);
+}
+
+GRUB_MOD_FINI
+{
+  clear_commands_list ();
+  grub_free (dirname);
+  
+  grub_unregister_command_autoloader (grub_autocmd_load_module_by_cmd);
+}
diff -rupN -x CVS -x '*.mk' grub2_savannah/normal/command.c grub2_autocmd/normal/command.c
--- grub2_savannah/normal/command.c	2004-09-20 09:23:31.000000000 +0200
+++ grub2_autocmd/normal/command.c	2004-09-27 21:38:19.000000000 +0200
@@ -26,6 +26,7 @@
 #include <grub/dl.h>
 
 static grub_command_t grub_command_list;
+static grub_command_autoloader_t grub_cmd_autoloader_list;
 
 void
 grub_register_command (const char *name,
@@ -71,9 +72,42 @@ grub_unregister_command (const char *nam
   for (p = &grub_command_list, q = *p; q; p = &(q->next), q = q->next)
     if (grub_strcmp (name, q->name) == 0)
       {
-        *p = q->next;
-        grub_free (q);
-        break;
+	*p = q->next;
+	grub_free (q);
+	break;
+      }
+}
+
+void
+grub_register_command_autoloader (grub_dl_t (*hook) (const char * cmd))
+{
+  grub_command_autoloader_t func, *p;
+
+  func = (grub_command_autoloader_t) grub_malloc (sizeof (*func));
+  if (! func)
+    return;
+
+  func->func = hook;
+  func->next = 0;
+
+  /* Probe with autoloaders in order in which they were loaded.  */
+  p = &grub_cmd_autoloader_list;
+  while (*p)
+    p = &((*p)->next);
+  *p = func;
+}
+
+void
+grub_unregister_command_autoloader (grub_dl_t (*hook) (const char * cmd))
+{
+  grub_command_autoloader_t *p, q;
+
+  for (p = &grub_cmd_autoloader_list, q = *p; q; p = &(q->next), q = q->next)
+    if (q->func == hook)
+      {
+	*p = q->next;
+	grub_free (q);
+	break;
       }
 }
 
@@ -92,6 +126,30 @@ grub_command_find (char *cmdline)
       break;
 
   if (! cmd)
+    {
+      grub_command_autoloader_t p;
+      grub_dl_t mod;
+
+      for (p = grub_cmd_autoloader_list; p; p = p->next)
+	{
+	  mod = p->func (cmdline);
+	  if (!mod)
+	    continue;
+
+	  for (cmd = grub_command_list; cmd; cmd = cmd->next)
+	    if (grub_strcmp (cmdline, cmd->name) == 0)
+	      break;
+
+	  if (cmd)
+	    break;
+
+	  grub_dl_unload (mod);
+	  mod = 0;
+	}
+      /*FIXME ref the mod?*/
+    }
+
+  if (! cmd)
     grub_error (GRUB_ERR_UNKNOWN_COMMAND, "unknown command `%s'", cmdline);
   
   if (first_space)
======================================================
-- 
                                 Tomas 'ebi' Ebenlendr
                                 http://get.to/ebik
                                 PF 2004.74280054645

Re: Automagic command loading

[Marco Gerards]

Tomas Ebenlendr <ebik@artax.karlin.mff.cuni.cz> writes:

> Here is new patch. The commands list is stored in elf section
> .uinfo.norm_cmds. The module searching mechanism is in autocmd.mod,
> so you switch on autoloading by inserting this module. Interface
> is designed such that there may be other modules autoregistering
> commands.
>
> The contents of .uinfo.norm_cmds of all modules in $prefix gets
> cached on insert of autocmd.mod, because reading of all modules takes
> about 5 seconds on my bochs under Linux, 300Mhz Pentium. The contents
> is recached by command cache_autocommands or when $prefix does not
> match cached directory. This will be used on very rare ocasions.

Does that mean that GRUB hangs for 5 seconds on boot when autocmd is
used?  I seriously doubt if that would be acceptable...  So is there
any way to speed this up, I think no one want to wait 5 seconds when
GRUB is started...

Here are my comments on your patch.  A lot of comments are still
related to the GCS.  But I also have a lot of questions on how things
work.  Perhaps I can comment better on certain parts of the patch
after you explained this.

> 	conf/i386-pc.rmk (pkgdata_MODULES): New module autocmd.mod

Please end the sentence with a ".".

> 	gencmdlist.sh: New file. Greps NORMAL_COMMAND in sources and generates
> 	list of commands (c source).

Two spaces between sentences.  But just "New file." would be enough
already.  Explaining what something does should be done in comments in
the file.

> 	incude/grub/normal.h (NORMAL_COMMAND): New macro. Used for marking
> 	commands to appear in special elf section.

in a special elf section

> 	normal/command.c (grub_register_command_autoloader): New function.

You forgot the '*' here.

> 	(grub_unregister_command_autoloader): New function.
> 	(grub_command_find): Execute autoloaders when command not found.

was not found

> -      char pathname[grub_strlen (dirname) + grub_strlen (filename) + 1];
> +      char pathname[grub_strlen (dirname) + grub_strlen (filename) + 2];

Please document this change in the changelog entry.

> -  grub_register_command ("hello", grub_cmd_hello, GRUB_COMMAND_FLAG_BOTH,
> +  grub_register_command (NORMAL_COMMAND("hello"), grub_cmd_hello, GRUB_COMMAND_FLAG_BOTH,
>  			 "hello", "Say hello", 0);

Please make sure lines can be fully shown on 80x25 terminals.

> +/* The command autoloader description.  */
> +struct grub_command_autoloader

What is is used for?  How?

> +{
> +  /* The callback function.  */
> +  grub_dl_t (*func) (const char * command);

const char *command

What does this callback do?

> +void EXPORT_FUNC(grub_register_command_autoloader) (
> +			    grub_dl_t (*hook) (const char * cmd));
> +void EXPORT_FUNC(grub_unregister_command_autoloader) (
> +			    grub_dl_t (*hook) (const char * cmd));

Can you explain this as well?

> +#define MEMFAILED \
> +  ({\
> +    grub_error (GRUB_ERR_OUT_OF_MEMORY, "Out of memory");\
> +    goto failed;\
> +  })
> +
> +#define READFAILED \
> +  ({\
> +    grub_error (GRUB_ERR_FILE_READ_ERROR, "Error reading file");\
> +    goto failed;\
> +  })

Personally I don't like this and this is not required.  grub_malloc
sets the right error string and sets grub_errno.  The same is true for
any generic function.  So you should just have "goto failed"  where
you have either MEMFAILED or READFAILED.

If any of the functions you use don't do the right thing, it should be
fixed there.

> +/* Reads value of user information in file (elf - module).
> +   Don't forget to free() result.  */

Isn't that something that is obvious?  IMHO such things should not be
documented.  Or do you mean a malloc'ed buffer should be returned?  In
That case I would just say that.

> +grub_err_t
> +grub_dl_read_file_uinfo (const char *filename, const char * secname, char ** value, int * len)

In the case of pointers, please do not add those extra spaces.  Write:

grub_dl_read_file_uinfo (const char *filename, const char *secname, char **value, int *len)

> +  section = grub_malloc (grub_strlen (secname) + 8);

Where does this 8 come from?

> +  readbytes = grub_file_read (file,(char *) &e, sizeof (e));

A space after the comma.

> +  if ((readbytes < (signed) sizeof (e)) || (e.e_ehsize < readbytes) || (e.e_shentsize < (signed) sizeof (*s)))

This line is too long...

> +  readbytes = grub_file_read (file,(char *) s, wantbytes);

A space after the comma.

> +  if (0)
> +    {
> +failed:
> +      ret = grub_errno;
> +    }

This looks like something you used when debugging?  Can you please
clean this up?

> +char * dirname = 0;

Can you remove the space and make this static?

> +module_commands_t modules_list = 0;

Same here.

> +  module_commands_t p, q;

module_commands_t p;
module_commands_t q;

> +
> +  for (p = modules_list; p; p=q)

p = q

> +static grub_err_t
> +cache_uinfo_files (struct grub_arg_list *state __attribute__ ((unused)),
> +	       int argc __attribute__ ((unused)),
> +	       char **args __attribute__ ((unused)))

The function name does not make clear to me that it is a command.  I
just recognised it as such because of the arguments.

> +    {
> +      char * pathname = 0;

Space...

> +      if ((suffixidx <= 0) || (grub_strcmp (filename + suffixidx,".mod") != 0))

A space before ".mod".

> +
> +      if (0)
> +	{
> +ret:
> +	  grub_free (secbuf);
> +	}

Same comment as before.  Is this debug code?


> +  fs = grub_fs_probe (dev);
> +  path = grub_strchr (dirname, ')') + 1;
> +
> +  if (! path || ! fs || (path[0] != '/'))
> +      goto fail;
> +      
> +  (fs->dir) (dev, path, cache_module);

Please don't do it like this.  Just use grub_fs_dir.

> +      /* Test if module has in .uinfo.norm_cmds written the command name.  */
> +      s = p->commands;
> +      while (s - p->commands < p->commands_len)
> +	{
> +	  if (grub_strcmp (s, command) == 0)
> +	    break;
> +	  s += grub_strlen (s) + 1;
> +	}

What does it mean if .uinfo.norm_cmds is not found?

> +      if (s >= p->commands + p->commands_len)
> +	return 0;
> +
> +      /* Test that module is not loaded.  */
> +      suffixidx = grub_strlen (p->module) - 4;

Why "- 4"?

> +  /* Check if prefix is cached.  */
> +  path = grub_env_get ("prefix");

Isn't there a grub_get_prefix command?

> +      (dirname[len] && ((dirname[len] != '/') || dirname[len+1])))

len + 1

> +    cache_uinfo_files (0, 0, 0);

What is this?

> +  /* Try <commandname>.mod.  */
> +  len = grub_strlen (command);
> +  modname = grub_malloc (len + 5);

Where is this memory freed?

> +void
> +grub_register_command_autoloader (grub_dl_t (*hook) (const char *
> +cmd))

So if I understand it correctly you made this generic.  You can
register an autoloader that is responsible for looking up modules.  In
that case it is easy to replace or could be not present at all.

Can you have multiple autoloaders?

Can you elaborate on the design of the autoloader?  So explain us how
an autoloader works, how it is used, the interfaces, etc?

> +{
> +  grub_command_autoloader_t func, *p;

Please don't declare multiple variable on one line.

> +grub_unregister_command_autoloader (grub_dl_t (*hook) (const char *
> +cmd))

I am not happy with the prefix.  You can better use the
"grub_autoloader_" prefix.  Not only here but everywhere where this is
logical.

> +  grub_command_autoloader_t *p, q;

Same here.


Two questions related to the cache:

- Where is it cached?

- How will the cache be depleted when autocmd is unloaded?

Thanks,
Marco

Re: Automagic command loading

[Tomas Ebenlendr]

> Does that mean that GRUB hangs for 5 seconds on boot when autocmd is
> used?  I seriously doubt if that would be acceptable...  So is there
> any way to speed this up, I think no one want to wait 5 seconds when
> GRUB is started...
> 

5 seconds on *very* slow machine. Probably caused by opening files by
full path. (I should figure out what is the slowest operation.)

> > +/* The command autoloader description.  */
> > +struct grub_command_autoloader
> 
> What is is used for?  How?
> 

Ok, i will add more comments. (This is used for storing a list of
command autoloader functions. (The module autocmd implements one such
function.))

> > +{
> > +  /* The callback function.  */
> > +  grub_dl_t (*func) (const char * command);
> 
> const char *command
> 
> What does this callback do?

O sorry, cut and pasted comment. This is the autoloading function.

> 
> > +void EXPORT_FUNC(grub_register_command_autoloader) (
> > +			    grub_dl_t (*hook) (const char * cmd));
> > +void EXPORT_FUNC(grub_unregister_command_autoloader) (
> > +			    grub_dl_t (*hook) (const char * cmd));
> 
> Can you explain this as well?
>
O.k. I'l add more comments in patch.
When command does not exist, the 'hook' is called. There can be
registered more hooks, they are executed in order they were registered.
Ouch, the return value of the hook is bad. Meaning should be: false -
command was not autoloaded (try next hook). True - command autoloaded.

> > +/* Reads value of user information in file (elf - module).
> > +   Don't forget to free() result.  */
> 
> Isn't that something that is obvious?  IMHO such things should not be
> documented.  Or do you mean a malloc'ed buffer should be returned?  In
> That case I would just say that.
>

Yes malloc'ed buffer is returned by following function. I'll chacnge the
comment.

> In the case of pointers, please do not add those extra spaces.

That is my bad custom. I'll try not to do that.

> > +  section = grub_malloc (grub_strlen (secname) + 8);
> 
> Where does this 8 come from?
> 

".uinfo." string is prepended.

> > +  if (0)
> > +    {
> > +failed:
> > +      ret = grub_errno;
> > +    }
> 
> This looks like something you used when debugging?  Can you please
> clean this up?
> 

This construction I use when I want to say do following statement only
if 'goto failed' was used, then continue with common code. Should I
duplicate the common code (about 5 lines of destruction), or should I
jump backwards (rename failed to ret, and add after return:
failed: ret = grub_errno; goto ret;)?

> > +static grub_err_t
> > +cache_uinfo_files (struct grub_arg_list *state __attribute__ ((unused)),
> > +	       int argc __attribute__ ((unused)),
> > +	       char **args __attribute__ ((unused)))
> 
> The function name does not make clear to me that it is a command.  I
> just recognised it as such because of the arguments.
> 

It is used both ways. (I call it manually with (0, 0, 0) atguments.)

> > +  fs = grub_fs_probe (dev);
> > +  path = grub_strchr (dirname, ')') + 1;
> > +
> > +  if (! path || ! fs || (path[0] != '/'))
> > +      goto fail;
> > +      
> > +  (fs->dir) (dev, path, cache_module);
> 
> Please don't do it like this.  Just use grub_fs_dir.
> 

Oh, I was copying the 'ls' command code. I didn't know this function.

> 
> What does it mean if .uinfo.norm_cmds is not found?
> 
The same as when empty: No command is provided by the module.

> > +      if (s >= p->commands + p->commands_len)
> > +	return 0;
> > +
> > +      /* Test that module is not loaded.  */
> > +      suffixidx = grub_strlen (p->module) - 4;
> 
> Why "- 4"?
> 

".mod"
I should probably use sizeof(".mod").

> > +  /* Check if prefix is cached.  */
> > +  path = grub_env_get ("prefix");
> 
> Isn't there a grub_get_prefix command?
>

Maybe. Again, this code I saw in dl.c and so I copied it.

> > +    cache_uinfo_files (0, 0, 0);
> 
> What is this?
> 

I just call the function that is also normal mode command. I don't like
function wrappers here.


> > +  /* Try <commandname>.mod.  */
> > +  len = grub_strlen (command);
> > +  modname = grub_malloc (len + 5);
> 
> Where is this memory freed?
> 

Bug. Thanks.

> > +void
> > +grub_register_command_autoloader (grub_dl_t (*hook) (const char *
> > +cmd))
> 
> So if I understand it correctly you made this generic.  You can
> register an autoloader that is responsible for looking up modules.  In
> that case it is easy to replace or could be not present at all.
> 
> Can you have multiple autoloaders?
> 

Yes they are called in order they were registered.

> Can you elaborate on the design of the autoloader?  So explain us how
> an autoloader works, how it is used, the interfaces, etc?
> 

Ok. I'll write it somewhere. (My disk is broken now, so It will take
some time.)

> Two questions related to the cache:
> 
> - Where is it cached?
> 

In autocmd.mod, there is connected list of pairs module name (with
suffix), .uinfo.norm_cmds content.

> - How will the cache be depleted when autocmd is unloaded?
>
Cache is cleared (and dealocated) on unloading, and also on recaching.
(But when recaching, it is immadiately allocated again and filled in.)

-- 
                                 Tomas 'ebi' Ebenlendr
                                 http://get.to/ebik
                                 PF 2004.74586052925

Re: Automagic command loading

[Yoshinori K. Okuji]

On Tuesday 28 September 2004 21:05, Tomas Ebenlendr wrote:
> Here is new patch. The commands list is stored in elf section
> .uinfo.norm_cmds. The module searching mechanism is in autocmd.mod,
> so you switch on autoloading by inserting this module. Interface
> is designed such that there may be other modules autoregistering
> commands.

I don't really understand why this solution is better than autocmd.lst. 
Can you summerize what are the problems in autocmd.lst and why this is 
superior?

Okuji

Re: Automagic command loading

[Tomas Ebenlendr]

> I don't really understand why this solution is better than autocmd.lst. 
> Can you summerize what are the problems in autocmd.lst and why this is 
> superior?
> 
> Okuji

I wanted to try this solution.

Advantage: no need of generating autocmd.lst, and editing this file by
user, when he adds his own module to distro-precompiled grub.

Disadvantage: current solution slowly reads the contens of directory.
This is probably caused by opening files by /full/ path, where we
principially have opened directory (and resolved path to it.).

Not so disadvanyage because:
1.) this solution caches the contents of directory in $prefix. So it is
slow only when inserting autocmd or after $prefix is changed. (or user
executed cache_autocommands - now i think the caching at insert of module
is misfeature. User can use cache_autocommands manually).
2.) Latency was under bochs on slow (P300) machine.

You can always turn it off by not inserting module autocmd.

-- 
                                 Tomas 'ebi' Ebenlendr
                                 http://get.to/ebik
                                 PF 2004.7458360213

Re: Automagic command loading

[Marco Gerards]

Tomas Ebenlendr <ebik@artax.karlin.mff.cuni.cz> writes:

> Disadvantage: current solution slowly reads the contens of directory.
> This is probably caused by opening files by /full/ path, where we
> principially have opened directory (and resolved path to it.).

Isn't it that slow because the *entire* ELF is read into memory?

> Not so disadvanyage because:
> 1.) this solution caches the contents of directory in $prefix. So it is
> slow only when inserting autocmd or after $prefix is changed. (or user
> executed cache_autocommands - now i think the caching at insert of module
> is misfeature. User can use cache_autocommands manually).

So how should it work in your opinion?  I think it should not cache
everything at startup.  It should look for "ls.mod" when you run
"ls".  If it can not be found it should try all modules until "ls" was
found.  The cache is filled automatically.

> 2.) Latency was under bochs on slow (P300) machine.
>
> You can always turn it off by not inserting module autocmd.

Or use another autoloader if I understand things correctly.

--
Marco

Re: Automagic command loading

[Tomas Ebenlendr]

> Tomas Ebenlendr <ebik@artax.karlin.mff.cuni.cz> writes:
> 
> > Disadvantage: current solution slowly reads the contens of directory.
> > This is probably caused by opening files by /full/ path, where we
> > principially have opened directory (and resolved path to it.).
> 
> Isn't it that slow because the *entire* ELF is read into memory?
> 

No, this was my filrst idea too. So I rewrited yhis part to read exactly
what I need (that is in the patch), and no speedup occured.

> > Not so disadvanyage because:
> > 1.) this solution caches the contents of directory in $prefix. So it is
> > slow only when inserting autocmd or after $prefix is changed. (or user
> > executed cache_autocommands - now i think the caching at insert of module
> > is misfeature. User can use cache_autocommands manually).
> 
> So how should it work in your opinion?  I think it should not cache
> everything at startup.  It should look for "ls.mod" when you run
> "ls".  If it can not be found it should try all modules until "ls" was
> found.  The cache is filled automatically.
> 

Hmm, that is not bad idea. I'll try to implement lazy caching. The busy
caching has an advantage of not reading the directory if command does
not exist, because cache is authoritative. (Loaded modules are cached
too, for the case they get unloaded.) But as I said, I didn't figured
out the slowest part yet...

> > 2.) Latency was under bochs on slow (P300) machine.
> >
> > You can always turn it off by not inserting module autocmd.
> 
> Or use another autoloader if I understand things correctly.
> 

Yes.

-- 
                                 Tomas 'ebi' Ebenlendr
                                 http://get.to/ebik
                                 PF 2004.74592924636

Re: Automagic command loading

[Yoshinori K. Okuji]

On Wednesday 29 September 2004 23:38, Tomas Ebenlendr wrote:
> Disadvantage: current solution slowly reads the contens of directory.
> This is probably caused by opening files by /full/ path, where we
> principially have opened directory (and resolved path to it.).

I think this is a critical problem. For example, if the user has a 
broken module which may crash GRUB, the user can easily break GRUB by 
trying a non-existent command.

Also, if my understanding is correct, the list of modules is stored in a 
binary form. This means that the user cannot (easily) modify the list 
after building GRUB.

The advantage of autocmd.lst is that it makes customization much easier. 
Suppose that the user wants to add a new command and build his own 
module. Then, how to autoload this command?

Okuji

Re: Automagic command loading

[Marco Gerards]

"Yoshinori K. Okuji" <okuji@enbug.org> writes:

> On Wednesday 29 September 2004 23:38, Tomas Ebenlendr wrote:
>> Disadvantage: current solution slowly reads the contens of directory.
>> This is probably caused by opening files by /full/ path, where we
>> principially have opened directory (and resolved path to it.).
>
> I think this is a critical problem. For example, if the user has a 
> broken module which may crash GRUB, the user can easily break GRUB by 
> trying a non-existent command.

As far as I understood what Tomas said, the modules are loaded and
only the new section is checked.  So it will not be initialized.

> Also, if my understanding is correct, the list of modules is stored in a 
> binary form. This means that the user cannot (easily) modify the list 
> after building GRUB.

Every module contains the list of commands it exports, AFAIK.  So the
is no single big central list.

> The advantage of autocmd.lst is that it makes customization much easier. 
> Suppose that the user wants to add a new command and build his own 
> module. Then, how to autoload this command?

The new module will contain this specific section as well.

--
Marco

Re: Automagic command loading

[Tomas Ebenlendr]

> "Yoshinori K. Okuji" <okuji@enbug.org> writes:
> 
> > On Wednesday 29 September 2004 23:38, Tomas Ebenlendr wrote:
> >> Disadvantage: current solution slowly reads the contens of directory.
> >> This is probably caused by opening files by /full/ path, where we
> >> principially have opened directory (and resolved path to it.).
> >
> > I think this is a critical problem. For example, if the user has a 
> > broken module which may crash GRUB, the user can easily break GRUB by 
> > trying a non-existent command.
> 
> As far as I understood what Tomas said, the modules are loaded and
> only the new section is checked.  So it will not be initialized.
> 
> > Also, if my understanding is correct, the list of modules is stored in a 
> > binary form. This means that the user cannot (easily) modify the list 
> > after building GRUB.
> 
> Every module contains the list of commands it exports, AFAIK.  So the
> is no single big central list.
> 
> > The advantage of autocmd.lst is that it makes customization much easier. 
> > Suppose that the user wants to add a new command and build his own 
> > module. Then, how to autoload this command?
> 
> The new module will contain this specific section as well.
> 

Yes, exactly. The section name is ".uinfo.norm_cmds" and command "command_name" is
stored there when NORMAL_COMMAND("command_name") occurs in file from
command_name_mod_SOURCES.

So typical use should be:

#ifdef GRUB_UTIL
module_name_init (...) {
    grub_register_command("command_name", ...)
}

#else
MODULE_INIT
{
    grub_register_command(NORMAL_COMMAND("command_name"), ...)
}
#endif

The first line with grub_register_command() does not affect module code.
So no NORMAL_COMMAND should be used. On the other hand, the second
line with grub_register_command() is code that occurs in module. So
using NORMAL_COMMAND ensures, that the "command_name" also occurs in the
special elf section.

-- 
                                 Tomas 'ebi' Ebenlendr
                                 http://get.to/ebik
                                 PF 2004.74813635271

Re: Automagic command loading

[Yoshinori K. Okuji]

My intuition is that this is not a good solution. So I think of another 
example.

The user does not like the behavior of, say, ls. So he writes his own 
module for an exhanced version of ls. Then, which does GRUB load?

I do not like this undeterministic behavior. You may think that the user 
can simply remove the original module and add his own. This is true, 
but I don't think this is a clean way.

One more example. Suppose that GRUB supports 100 commands and 100 
filesystems. Probably the overhead of loading all modules would be 
negligible on Pentium4, but isn't it significant on 486? We should not 
forget that some people still use very old computers.

I don't see any serious disadvantage in autocmd.lst. Generating this 
file is as easy as embedding command names in modules. Editing this 
file is not difficult at all for users who can write their own modules.

Generally speaking, text-based approaches are better than binary-based 
approaches. For example, if the user wants to know what modules 
contains a given command, he can just grep autocmd.lst instead of 
reading the source code or using readelf.

BTW, I think it would be useful to investigate why loading all modules 
takes 5 seconds on bochs. Even on bochs, I feel this is too much. There 
might be a bug in the disk cache system. Tomas, could you look at the 
cache efficiency when loading all modules? You can get this information 
by the function grub_disk_cache_get_performance. To enable this 
function, you need to change kern/disk.c, because I disable this by #if 
0 ... #endif.

Okuji

Re: Automagic command loading

[chaac]

Yoshinori K. Okuji wrote:
> I don't see any serious disadvantage in autocmd.lst. Generating this 
> file is as easy as embedding command names in modules. Editing this 
> file is not difficult at all for users who can write their own modules.

One advantage for autocmd.lst is that you can probably specify where to 
load module. Let's assume you are coding some module and want to store 
that module on network or another drive/path. Then you could just 
specify location to there.

Thanks,
Vesa Jääskeläinen

Re: Automagic command loading

[Tomas Ebenlendr]

Ok, I think that common opinion is that autocmd.lst is better. So I'll
write autogenerating it and reading it e.g. in autocmd.mod.

I also try to figure out the slow point in this solution.

But I have folowing questions:

    1.) Should I throw away current implementation, or should there be
    a module with this implementation (e.g. autocmd_elf.mod).

    2.) I think, that having information about supported commands in
    module is not bad. I can write tool, to read it (and generate
    autocmd.lst from this information). Advantage will be following:
    User1 compiles his own module and sends it to User2. User2 will copy
    the module on proper location and run:
	grub-mod-read-cmds the_module.mod >> autocmd.lst    

> My intuition is that this is not a good solution. So I think of another 
> example.
> 
> The user does not like the behavior of, say, ls. So he writes his own 
> module for an exhanced version of ls. Then, which does GRUB load?
> 
> I do not like this undeterministic behavior. You may think that the user 
> can simply remove the original module and add his own. This is true, 
> but I don't think this is a clean way.
> 
> One more example. Suppose that GRUB supports 100 commands and 100 
> filesystems. Probably the overhead of loading all modules would be 
> negligible on Pentium4, but isn't it significant on 486? We should not 
> forget that some people still use very old computers.
> 
> I don't see any serious disadvantage in autocmd.lst. Generating this 
> file is as easy as embedding command names in modules. Editing this 
> file is not difficult at all for users who can write their own modules.
> 
> Generally speaking, text-based approaches are better than binary-based 
> approaches. For example, if the user wants to know what modules 
> contains a given command, he can just grep autocmd.lst instead of 
> reading the source code or using readelf.
> 
> BTW, I think it would be useful to investigate why loading all modules 
> takes 5 seconds on bochs. Even on bochs, I feel this is too much. There 
> might be a bug in the disk cache system. Tomas, could you look at the 
> cache efficiency when loading all modules? You can get this information 
> by the function grub_disk_cache_get_performance. To enable this 
> function, you need to change kern/disk.c, because I disable this by #if 
> 0 ... #endif.
> 
> Okuji
> 
> 
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> http://lists.gnu.org/mailman/listinfo/grub-devel

-- 
                                 Tomas 'ebi' Ebenlendr
                                 http://get.to/ebik
                                 PF 2004.75133677393

Re: Automagic command loading

[Yoshinori K. Okuji]

On Friday 01 October 2004 23:56, Tomas Ebenlendr wrote:
> But I have folowing questions:
>
>     1.) Should I throw away current implementation, or should there
> be a module with this implementation (e.g. autocmd_elf.mod).

If you think it is useful, you can commit it. But I don't see any 
benefit if we have another implementation.

>     2.) I think, that having information about supported commands in
>     module is not bad. I can write tool, to read it (and generate
>     autocmd.lst from this information). Advantage will be following:
>     User1 compiles his own module and sends it to User2. User2 will
> copy the module on proper location and run:
> 	grub-mod-read-cmds the_module.mod >> autocmd.lst

It sounds to me the same as telling User2 this:

echo foo foo.mod >> autocmd.lst

But it is up to you how to build autocmd.lst. If you think it is easier 
to use an ELF section, why not. But I'm a bit afraid of a too much 
dependency upon ELF.

Okuji