Re: Bug#586358: grub-pc: Please handle DOS line endings in "cat"

[Colin Watson <cjwatson@ubuntu.com>]

On Fri, Jun 25, 2010 at 08:51:12PM +0200, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
> Colin Watson wrote:
> > On Fri, Jun 18, 2010 at 11:34:08AM -0700, Josh Triplett wrote:
> >> The "cat" command, if given a file with DOS line endings, will show the
> >> \r at the end of every line as a control character.  For convenience,
> >> please consider adding support for this in "cat", ideally automatically.
> >
> > Makes sense to me.  grub-devel, how about this patch?
> 
> Unconditionaly stripping \r is a bad idea because sometimes cat in grub
> is used to determine why the system fails to boot. And if by a stupid
> action fstab becomes DOS-formated 'cat /etc/fstab' will fail to show
> this important point. But it's ok to add an option for cat'ing DOS
> files.

OK.  How about the following patch, then?

> In this case we probably need to mark unix newlines somehow to
> detect a reverse problem

I think we can live without this.  It adds complexity (I can't
immediately think of a neat way to represent this visually) and I think
it would be needed quite rarely.  Besides, I was looking around for
examples and noticed that when vim is in DOS mode it simply treats both
\r\n and \n as newlines.  That seems a good enough precedent for me.

2010-06-20  Colin Watson  <cjwatson@ubuntu.com>

	* commands/cat.c (options): New variable.
	(grub_cmd_cat): Parse options.  If the --dos option is given, print
	DOS-style "\r\n" line endings as simple newlines.
	(GRUB_MOD_INIT): Use extcmd.
	(GRUB_MOD_FINI): Likewise.
	* docs/grub.texi (cat): Document --dos.

=== modified file 'commands/cat.c'
--- commands/cat.c	2010-03-01 19:24:34 +0000
+++ commands/cat.c	2010-06-27 23:56:11 +0000
@@ -23,19 +23,28 @@
 #include <grub/term.h>
 #include <grub/misc.h>
 #include <grub/gzio.h>
-#include <grub/command.h>
+#include <grub/extcmd.h>
 #include <grub/i18n.h>
 
-static grub_err_t
-grub_cmd_cat (grub_command_t cmd __attribute__ ((unused)),
-	      int argc, char **args)
+static const struct grub_arg_option options[] =
+  {
+    {"dos", -1, 0, N_("Accept DOS-style CR/NL line endings."), 0, 0},
+    {0, 0, 0, 0, 0, 0}
+  };
 
+static grub_err_t
+grub_cmd_cat (grub_extcmd_t cmd, int argc, char **args)
 {
+  struct grub_arg_list *state = cmd->state;
+  int dos = 0;
   grub_file_t file;
   char buf[GRUB_DISK_SECTOR_SIZE];
   grub_ssize_t size;
   int key = 0;
 
+  if (state[0].set)
+    dos = 1;
+
   if (argc != 1)
     return grub_error (GRUB_ERR_BAD_ARGUMENT, "file name required");
 
@@ -54,6 +63,11 @@ grub_cmd_cat (grub_command_t cmd __attri
 
 	  if ((grub_isprint (c) || grub_isspace (c)) && c != '\r')
 	    grub_putchar (c);
+	  else if (dos && c == '\r' && buf[i + 1] == '\n')
+	    {
+	      grub_putchar ('\n');
+	      i++;
+	    }
 	  else
 	    {
 	      grub_setcolorstate (GRUB_TERM_COLOR_HIGHLIGHT);
@@ -74,15 +88,16 @@ grub_cmd_cat (grub_command_t cmd __attri
   return 0;
 }
 
-static grub_command_t cmd;
+static grub_extcmd_t cmd;
 \f
 GRUB_MOD_INIT(cat)
 {
-  cmd = grub_register_command_p1 ("cat", grub_cmd_cat,
-				  N_("FILE"), N_("Show the contents of a file."));
+  cmd = grub_register_extcmd ("cat", grub_cmd_cat, GRUB_COMMAND_FLAG_BOTH,
+			      N_("FILE"), N_("Show the contents of a file."),
+			      options);
 }
 
 GRUB_MOD_FINI(cat)
 {
-  grub_unregister_command (cmd);
+  grub_unregister_extcmd (cmd);
 }

=== modified file 'docs/grub.texi'
--- docs/grub.texi	2010-06-26 09:50:39 +0000
+++ docs/grub.texi	2010-06-27 23:45:11 +0000
@@ -1624,13 +1624,19 @@ a menu entry).
 @node cat
 @subsection cat
 
-@deffn Command cat file
+@deffn Command cat [@option{--dos}] file
 Display the contents of the file @var{file}. This command may be useful
 to remind you of your OS's root partition:
 
 @example
 grub> @kbd{cat /etc/fstab}
 @end example
+
+If the @option{--dos} option is used, then carriage return / new line pairs
+will be displayed as a simple new line.  Otherwise, the carriage return will
+be displayed as a control character (@samp{<d>}) to make it easier to see
+when boot problems are caused by a file formatted using DOS-style line
+endings.
 @end deffn
 
 

Thanks,

-- 
Colin Watson                                       [cjwatson@ubuntu.com]