[PATCH] read command

[PATCH] read command

[Robert Millan]

[-- Attachment #1: Type: text/plain, Size: 184 bytes --]


Implements a BASH-like read command.

-- 
Robert Millan

<GPLv2> I know my rights; I want my phone call!
<DRM> What use is a phone call… if you are unable to speak?
(as seen on /.)

[-- Attachment #2: read.diff --]
[-- Type: text/x-diff, Size: 3236 bytes --]


	* commands/read.c: New file.
	* conf/common.rmk (pkglib_MODULES): Add `commands/read.c'.
	(read_mod_SOURCES): New variable.
	(read_mod_CFLAGS): Likewise.
	(read_mod_LDFLAGS): Likewise.

diff -x CVS -x '*~' -x '*.mk' -urp -N ../grub2/commands/read.c ./commands/read.c
--- ../grub2/commands/read.c	1970-01-01 01:00:00.000000000 +0100
+++ ./commands/read.c	2008-02-01 23:44:25.000000000 +0100
@@ -0,0 +1,77 @@
+/* read.c - Command to read variables from user.  */
+/*
+ *  GRUB  --  GRand Unified Bootloader
+ *  Copyright (C) 2006,2007,2008  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 3 of the License, or
+ *  (at your option) any later version.
+ *
+ *  GRUB 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, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <grub/dl.h>
+#include <grub/misc.h>
+#include <grub/mm.h>
+#include <grub/normal.h>
+#include <grub/term.h>
+#include <grub/types.h>
+
+static void *
+grub_xrealloc (void *ptr, grub_size_t size)
+{
+  void *value = grub_realloc (ptr, size);
+  if (value == 0)
+    grub_fatal ("Virtual memory exhausted");
+  return value;
+}
+
+static char *
+grub_getline (void)
+{
+  int i;
+  char *line;
+
+  i = 0;
+  line = grub_malloc (1 + i + sizeof('\0'));
+
+  while ((line[i - 1] != '\n') && (line[i - 1] != '\r'))
+    {
+      line[i] = grub_getkey ();
+      if (grub_isprint (line[i]))
+	grub_putchar (line[i]);
+      i++;
+      line = grub_xrealloc (line, 1 + i + sizeof('\0'));
+    }
+  line[i] = '\0';
+
+  return line;
+}
+
+static grub_err_t
+grub_cmd_read (struct grub_arg_list *state UNUSED, int argc, char **args)
+{
+  char *line = grub_getline ();
+  if (argc > 0)
+    grub_env_set (args[0], line);
+  return 0;
+}
+
+\f
+GRUB_MOD_INIT(read)
+{
+  grub_register_command ("read", grub_cmd_read, GRUB_COMMAND_FLAG_CMDLINE,
+			 "read [ENVVAR]", "Set variable with user input", 0);
+}
+
+GRUB_MOD_FINI(read)
+{
+  grub_unregister_command ("read");
+}
diff -x CVS -x '*~' -x '*.mk' -urp -N ../grub2/conf/common.rmk ./conf/common.rmk
--- ../grub2/conf/common.rmk	2008-01-30 16:32:55.000000000 +0100
+++ ./conf/common.rmk	2008-02-01 22:12:02.000000000 +0100
@@ -231,7 +231,8 @@ lvm_mod_LDFLAGS = $(COMMON_LDFLAGS)
 pkglib_MODULES += hello.mod boot.mod terminal.mod ls.mod	\
 	cmp.mod cat.mod help.mod font.mod search.mod		\
 	loopback.mod configfile.mod echo.mod			\
-	terminfo.mod test.mod blocklist.mod hexdump.mod
+	terminfo.mod test.mod blocklist.mod hexdump.mod		\
+	read.mod
 
 # For hello.mod.
 hello_mod_SOURCES = hello/hello.c
@@ -326,5 +327,7 @@ gzio_mod_SOURCES = io/gzio.c
 gzio_mod_CFLAGS = $(COMMON_CFLAGS)
 gzio_mod_LDFLAGS = $(COMMON_LDFLAGS)
 
-
-
+# For read.mod.
+read_mod_SOURCES = commands/read.c
+read_mod_CFLAGS = $(COMMON_CFLAGS)
+read_mod_LDFLAGS = $(COMMON_LDFLAGS)

Re: [PATCH] read command

[Robert Millan]

On Fri, Feb 01, 2008 at 11:45:50PM +0100, Robert Millan wrote:
> +static void *
> +grub_xrealloc (void *ptr, grub_size_t size)
> +{
> +  void *value = grub_realloc (ptr, size);
> +  if (value == 0)
> +    grub_fatal ("Virtual memory exhausted");
> +  return value;
> +}
> +
> +static char *
> +grub_getline (void)
> +{
> +  int i;
> +  char *line;
> +
> +  i = 0;
> +  line = grub_malloc (1 + i + sizeof('\0'));
> +
> +  while ((line[i - 1] != '\n') && (line[i - 1] != '\r'))
> +    {
> +      line[i] = grub_getkey ();
> +      if (grub_isprint (line[i]))
> +	grub_putchar (line[i]);
> +      i++;
> +      line = grub_xrealloc (line, 1 + i + sizeof('\0'));
> +    }
> +  line[i] = '\0';
> +
> +  return line;
> +}

Does it make sense to move any of these two to kernel?  Or to normal.mod ?  Or
maybe just to un-static-ize them and leave them here?

-- 
Robert Millan

<GPLv2> I know my rights; I want my phone call!
<DRM> What use is a phone call… if you are unable to speak?
(as seen on /.)

Re: [PATCH] read command

[Yoshinori K. Okuji]

On Friday 01 February 2008 23:48, Robert Millan wrote:
> On Fri, Feb 01, 2008 at 11:45:50PM +0100, Robert Millan wrote:
> > +static void *
> > +grub_xrealloc (void *ptr, grub_size_t size)
> > +{
> > +  void *value = grub_realloc (ptr, size);
> > +  if (value == 0)
> > +    grub_fatal ("Virtual memory exhausted");
> > +  return value;
> > +}
> > +
> > +static char *
> > +grub_getline (void)
> > +{
> > +  int i;
> > +  char *line;
> > +
> > +  i = 0;
> > +  line = grub_malloc (1 + i + sizeof('\0'));
> > +
> > +  while ((line[i - 1] != '\n') && (line[i - 1] != '\r'))
> > +    {
> > +      line[i] = grub_getkey ();
> > +      if (grub_isprint (line[i]))
> > +	grub_putchar (line[i]);
> > +      i++;
> > +      line = grub_xrealloc (line, 1 + i + sizeof('\0'));
> > +    }
> > +  line[i] = '\0';
> > +
> > +  return line;
> > +}
>
> Does it make sense to move any of these two to kernel?  Or to normal.mod ? 
> Or maybe just to un-static-ize them and leave them here?

What would be other use cases?

BTW it is a bad idea to use xrealloc. xmalloc and xrealloc are sometimes used 
in the GNU Project, but they must not be used in GRUB. If a program runs in 
user space, when it panics, the control is back to the OS. But GRUB is a 
standalone program. When it panics, the user loses any kind of control. Thus 
grub_fatal must be used only if there is nothing else you can do.

In this case, grub_getline should simply return NULL. How the user deals with 
this situation is up to the user.

Okuji

Re: [PATCH] read command

[Robert Millan]

[-- Attachment #1: Type: text/plain, Size: 1147 bytes --]

On Sat, Feb 02, 2008 at 01:38:28PM +0100, Yoshinori K. Okuji wrote:
> >
> > Does it make sense to move any of these two to kernel?  Or to normal.mod ? 
> > Or maybe just to un-static-ize them and leave them here?
> 
> What would be other use cases?

No idea.  I suggest we just leave grub_getline here and if later we find it's
needed we can move it to normal or kernel.

> BTW it is a bad idea to use xrealloc. xmalloc and xrealloc are sometimes used 
> in the GNU Project, but they must not be used in GRUB. If a program runs in 
> user space, when it panics, the control is back to the OS. But GRUB is a 
> standalone program. When it panics, the user loses any kind of control. Thus 
> grub_fatal must be used only if there is nothing else you can do.
> 
> In this case, grub_getline should simply return NULL. How the user deals with 
> this situation is up to the user.

Ok, here's a new patch, with grub_getline returning NULL and the caller handling
"out of mem" error.

Also fixed a memleak.

-- 
Robert Millan

<GPLv2> I know my rights; I want my phone call!
<DRM> What use is a phone call… if you are unable to speak?
(as seen on /.)

[-- Attachment #2: read.diff --]
[-- Type: text/x-diff, Size: 3243 bytes --]


	* commands/read.c: New file.
	* conf/common.rmk (pkglib_MODULES): Add `commands/read.c'.
	(read_mod_SOURCES): New variable.
	(read_mod_CFLAGS): Likewise.
	(read_mod_LDFLAGS): Likewise.

diff -x CVS -x '*~' -x '*.mk' -urp -N ../grub2/commands/read.c ./commands/read.c
--- ../grub2/commands/read.c	1970-01-01 01:00:00.000000000 +0100
+++ ./commands/read.c	2008-02-02 18:02:31.000000000 +0100
@@ -0,0 +1,79 @@
+/* read.c - Command to read variables from user.  */
+/*
+ *  GRUB  --  GRand Unified Bootloader
+ *  Copyright (C) 2006,2007,2008  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 3 of the License, or
+ *  (at your option) any later version.
+ *
+ *  GRUB 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, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <grub/dl.h>
+#include <grub/misc.h>
+#include <grub/mm.h>
+#include <grub/normal.h>
+#include <grub/term.h>
+#include <grub/types.h>
+
+static char *
+grub_getline (void)
+{
+  int i;
+  char *line;
+  char *tmp;
+
+  i = 0;
+  line = grub_malloc (1 + i + sizeof('\0'));
+
+  while ((line[i - 1] != '\n') && (line[i - 1] != '\r'))
+    {
+      line[i] = grub_getkey ();
+      if (grub_isprint (line[i]))
+	grub_putchar (line[i]);
+      i++;
+      tmp = grub_realloc (line, 1 + i + sizeof('\0'));
+      if (! tmp)
+	{
+	  grub_free (line);
+	  return NULL;
+	}
+      line = tmp;
+    }
+  line[i] = '\0';
+
+  return line;
+}
+
+static grub_err_t
+grub_cmd_read (struct grub_arg_list *state UNUSED, int argc, char **args)
+{
+  char *line = grub_getline ();
+  if (! line)
+    return grub_error (GRUB_ERR_OUT_OF_MEMORY, "Out of memory");
+  if (argc > 0)
+    grub_env_set (args[0], line);
+
+  grub_free (line);
+  return 0;
+}
+
+\f
+GRUB_MOD_INIT(read)
+{
+  grub_register_command ("read", grub_cmd_read, GRUB_COMMAND_FLAG_CMDLINE,
+			 "read [ENVVAR]", "Set variable with user input", 0);
+}
+
+GRUB_MOD_FINI(read)
+{
+  grub_unregister_command ("read");
+}
diff -x CVS -x '*~' -x '*.mk' -urp -N ../grub2/conf/common.rmk ./conf/common.rmk
--- ../grub2/conf/common.rmk	2008-01-30 16:32:55.000000000 +0100
+++ ./conf/common.rmk	2008-02-02 17:54:35.000000000 +0100
@@ -231,7 +231,8 @@ lvm_mod_LDFLAGS = $(COMMON_LDFLAGS)
 pkglib_MODULES += hello.mod boot.mod terminal.mod ls.mod	\
 	cmp.mod cat.mod help.mod font.mod search.mod		\
 	loopback.mod configfile.mod echo.mod			\
-	terminfo.mod test.mod blocklist.mod hexdump.mod
+	terminfo.mod test.mod blocklist.mod hexdump.mod		\
+	read.mod
 
 # For hello.mod.
 hello_mod_SOURCES = hello/hello.c
@@ -326,5 +327,7 @@ gzio_mod_SOURCES = io/gzio.c
 gzio_mod_CFLAGS = $(COMMON_CFLAGS)
 gzio_mod_LDFLAGS = $(COMMON_LDFLAGS)
 
-
-
+# For read.mod.
+read_mod_SOURCES = commands/read.c
+read_mod_CFLAGS = $(COMMON_CFLAGS)
+read_mod_LDFLAGS = $(COMMON_LDFLAGS)

Re: [PATCH] read command

[Yoshinori K. Okuji]

On Saturday 02 February 2008 18:05, Robert Millan wrote:
> On Sat, Feb 02, 2008 at 01:38:28PM +0100, Yoshinori K. Okuji wrote:
> > > Does it make sense to move any of these two to kernel?  Or to
> > > normal.mod ? Or maybe just to un-static-ize them and leave them here?
> >
> > What would be other use cases?
>
> No idea.  I suggest we just leave grub_getline here and if later we find
> it's needed we can move it to normal or kernel.

OK.

> Ok, here's a new patch, with grub_getline returning NULL and the caller
> handling "out of mem" error.
>
> Also fixed a memleak.

You don't have to set grub_errno explicitly, because grub_realloc should do 
it.

And, you didn't check the return value from grub_malloc.

Okuji

Re: [PATCH] read command

[Robert Millan]

On Sat, Feb 02, 2008 at 07:48:57PM +0100, Yoshinori K. Okuji wrote:
> > Ok, here's a new patch, with grub_getline returning NULL and the caller
> > handling "out of mem" error.
> >
> > Also fixed a memleak.
> 
> You don't have to set grub_errno explicitly, because grub_realloc should do 
> it.
> 
> And, you didn't check the return value from grub_malloc.

Fixed and committed.

-- 
Robert Millan

<GPLv2> I know my rights; I want my phone call!
<DRM> What use is a phone call… if you are unable to speak?
(as seen on /.)