[PATCH] Don't open same disk twice on OpenFirmware.

2009-12-18 Thread Vladimir 'φ-coder/phcoder' Serbinenko
Hello, all. According to David Miller sparc's openboot doesn't support
opening same disk twice. So I implemented handle reusage logic. Tested
on imac g3

-- 
Regards
Vladimir 'φ-coder/phcoder' Serbinenko

=== modified file 'disk/ieee1275/ofdisk.c'
--- disk/ieee1275/ofdisk.c	2009-12-07 10:54:25 +
+++ disk/ieee1275/ofdisk.c	2009-12-18 16:12:28 +
@@ -1,7 +1,7 @@
 /* ofdisk.c - Open Firmware disk access.  */
 /*
  *  GRUB  --  GRand Unified Bootloader
- *  Copyright (C) 2004,2006,2007,2008  Free Software Foundation, Inc.
+ *  Copyright (C) 2004,2006,2007,2008,2009  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
@@ -26,6 +26,8 @@
 struct ofdisk_hash_ent
 {
   char *devpath;
+  int refs;
+  grub_ieee1275_ihandle_t dev_ihandle;
   struct ofdisk_hash_ent *next;
 };
 
@@ -65,6 +67,8 @@ ofdisk_hash_add (char *devpath)
 {
   p-devpath = devpath;
   p-next = *head;
+  p-refs = 0;
+  p-dev_ihandle = 0;
   *head = p;
 }
   return p;
@@ -170,6 +174,23 @@ grub_ofdisk_open (const char *name, grub
   if (!op)
 return grub_errno;
 
+  if (op-dev_ihandle)
+{
+  op-refs++;
+
+  /* XXX: There is no property to read the number of blocks.  There
+	 should be a property `#blocks', but it is not there.  Perhaps it
+	 is possible to use seek for this.  */
+  disk-total_sectors = 0xUL;
+
+  disk-id = (unsigned long) op;
+
+  /* XXX: Read this, somehow.  */
+  disk-has_partitions = 1;
+  disk-data = op;
+  return 0;
+}
+
   grub_dprintf (disk, Opening `%s'.\n, op-devpath);
 
   grub_ieee1275_open (op-devpath, dev_ihandle);
@@ -179,8 +200,8 @@ grub_ofdisk_open (const char *name, grub
   goto fail;
 }
 
-  grub_dprintf (disk, Opened `%s' as handle %p.\n, op-devpath,
-		(void *) (unsigned long) dev_ihandle);
+  grub_dprintf (disk, Opened `%s' as handle 0x%lx.\n, op-devpath,
+		(unsigned long) dev_ihandle);
 
   if (grub_ieee1275_finddevice (op-devpath, dev))
 {
@@ -201,6 +222,9 @@ grub_ofdisk_open (const char *name, grub
   goto fail;
 }
 
+  op-dev_ihandle = dev_ihandle;
+  op-refs++;
+
   /* XXX: There is no property to read the number of blocks.  There
  should be a property `#blocks', but it is not there.  Perhaps it
  is possible to use seek for this.  */
@@ -210,7 +234,7 @@ grub_ofdisk_open (const char *name, grub
 
   /* XXX: Read this, somehow.  */
   disk-has_partitions = 1;
-  disk-data = (void *) (unsigned long) dev_ihandle;
+  disk-data = op;
   return 0;
 
  fail:
@@ -222,9 +246,15 @@ grub_ofdisk_open (const char *name, grub
 static void
 grub_ofdisk_close (grub_disk_t disk)
 {
-  grub_dprintf (disk, Closing handle %p.\n,
-		(void *) disk-data);
-  grub_ieee1275_close ((grub_ieee1275_ihandle_t) (unsigned long) disk-data);
+  struct ofdisk_hash_ent *data = disk-data;
+
+  data-refs--;
+  if (data-refs)
+return;
+
+  grub_dprintf (disk, Closing handle %p.\n, data);
+  grub_ieee1275_close (data-dev_ihandle);
+  data-dev_ihandle = 0;
 }
 
 static grub_err_t
@@ -233,21 +263,21 @@ grub_ofdisk_read (grub_disk_t disk, grub
 {
   grub_ssize_t status, actual;
   unsigned long long pos;
+  struct ofdisk_hash_ent *data = disk-data;
 
   grub_dprintf (disk,
 		Reading handle %p: sector 0x%llx, size 0x%lx, buf %p.\n,
-		(void *) disk-data, (long long) sector, (long) size, buf);
+		data, (long long) sector, (long) size, buf);
 
   pos = sector * 512UL;
 
-  grub_ieee1275_seek ((grub_ieee1275_ihandle_t) (unsigned long) disk-data,
+  grub_ieee1275_seek (data-dev_ihandle,
 		  (int) (pos  32), (int) pos  0xUL, status);
   if (status  0)
 return grub_error (GRUB_ERR_READ_ERROR,
 		   Seek error, can't seek block %llu,
 		   (long long) sector);
-  grub_ieee1275_read ((grub_ieee1275_ihandle_t) (unsigned long) disk-data,
-		  buf, size * 512UL, actual);
+  grub_ieee1275_read (data-dev_ihandle, buf, size * 512UL, actual);
   if (actual != actual)
 return grub_error (GRUB_ERR_READ_ERROR, Read error on block: %llu,
 		   (long long) sector);



signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH] Don't open same disk twice on OpenFirmware.

2009-12-18 Thread David Miller
From: Vladimir 'φ-coder/phcoder' Serbinenko phco...@gmail.com
Date: Fri, 18 Dec 2009 17:17:13 +0100

 Hello, all. According to David Miller sparc's openboot doesn't support
 opening same disk twice. So I implemented handle reusage logic. Tested
 on imac g3

At a minimum you have to seek to the beginning of the device
as that is one of the many side effects of openning the
device.

I really don't think this is a good idea, really.

We sould just enforce the restriction, and put a debugging
check there to make sure the invariant is never violated.
___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH] Don't open same disk twice on OpenFirmware.

2009-12-18 Thread David Miller
From: David Miller da...@davemloft.net
Date: Fri, 18 Dec 2009 10:33:13 -0800 (PST)

 From: Vladimir 'φ-coder/phcoder' Serbinenko phco...@gmail.com
 Date: Fri, 18 Dec 2009 17:17:13 +0100
 
 Hello, all. According to David Miller sparc's openboot doesn't support
 opening same disk twice. So I implemented handle reusage logic. Tested
 on imac g3
 
 At a minimum you have to seek to the beginning of the device
 as that is one of the many side effects of openning the
 device.

In fact this proves that your idea can't ever work.
Each openned instance will expect the file offset
pointer to be at different locations.

You can't share open instances, therefore.

We really just have to make sure GRUB never violates
this restriction.
___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH] Don't open same disk twice on OpenFirmware.

2009-12-18 Thread Vladimir 'φ-coder/phcoder' Serbinenko
David Miller wrote:
 From: David Miller da...@davemloft.net
 Date: Fri, 18 Dec 2009 10:33:13 -0800 (PST)

   
 From: Vladimir 'φ-coder/phcoder' Serbinenko phco...@gmail.com
 Date: Fri, 18 Dec 2009 17:17:13 +0100

 
 Hello, all. According to David Miller sparc's openboot doesn't support
 opening same disk twice. So I implemented handle reusage logic. Tested
 on imac g3
   
 At a minimum you have to seek to the beginning of the device
 as that is one of the many side effects of openning the
 device.
 

 In fact this proves that your idea can't ever work.
 Each openned instance will expect the file offset
 pointer to be at different locations.

 You can't share open instances, therefore.

 We really just have to make sure GRUB never violates
 this restriction.
   
Actually we don't open files, only disks and we explicitely seek to
desired position. It was so even before my patch.

-- 
Regards
Vladimir 'φ-coder/phcoder' Serbinenko




signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: Unittests

2009-12-18 Thread Carles Pina i Estany

Hi,

On Dec/17/2009, BVK Chaitanya wrote:

 I attached a new patch, which has an example gettext_1.in testcase --
 it doesn't yet do what you wanted, but is the starter.
 
 Let me know your comments.

When I will check I will tell you something :-) (I will do it in some
days/during holidays...)

Thanks,

-- 
Carles Pina i Estany
http://pinux.info


___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


gettext: version string in normal/main.c

2009-12-18 Thread Carles Pina i Estany

Hi,

I've gettextitzzed the version string.

Something to improve before committing?
It's all right?

Thanks,

-- 
Carles Pina i Estany
http://pinux.info
=== modified file 'ChangeLog'
--- ChangeLog	2009-12-19 00:05:41 +
+++ ChangeLog	2009-12-19 01:27:34 +
@@ -1,5 +1,18 @@
 2009-12-19  Carles Pina i Estany  car...@pina.cat
 
+	* include/grub/normal.h (utf8_to_ucs4): New declaration.
+	(grub_print_ucs4): Likewise.
+	(getstringwidth): Likewise.
+	* normal/main.c (grub_normal_init_page): Gettextize version string.
+	* normal/menu_text.c (utf8_to_ucs4): New declaration.
+	(getstringwidth): Remove `static' qualifier (now used in
+	normal/main.c).  Use `utf8_to_ucs4'.
+	(grub_print_ucs4): Remove `static' qualifer (now used in
+	normal/main.c).
+	* po/POTFILES: Add normal/main.c.
+
+2009-12-19  Carles Pina i Estany  car...@pina.cat
+
 	* normal/menu_text.c (STANDARD_MARGIN): New macro.
 	(print_message_indented): Add `margin_left' and `margin_right'
 	parameters.

=== modified file 'include/grub/normal.h'
--- include/grub/normal.h	2009-08-24 23:55:06 +
+++ include/grub/normal.h	2009-12-19 01:21:13 +
@@ -73,6 +73,12 @@ void grub_parse_color_name_pair (grub_ui
 
 /* Defined in `menu_text.c'.  */
 void grub_wait_after_message (void);
+int utf8_to_ucs4 (const char *msg, grub_uint32_t **unicode_msg,
+			grub_uint32_t **last_position);
+void grub_print_ucs4 (const grub_uint32_t * str,
+			const grub_uint32_t * last_position);
+grub_ssize_t getstringwidth (grub_uint32_t * str,
+			const grub_uint32_t * last_position);
 
 /* Defined in `handler.c'.  */
 void read_handler_list (void);

=== modified file 'normal/main.c'
--- normal/main.c	2009-12-08 00:08:52 +
+++ normal/main.c	2009-12-19 01:36:39 +
@@ -385,22 +385,31 @@ read_config_file (const char *config)
 void
 grub_normal_init_page (void)
 {
-  grub_uint8_t width, margin;
+  grub_cls ();
+  const char *msg = _(GNU GRUB  version %s);
 
-#define TITLE (GNU GRUB  version  PACKAGE_VERSION)
+  char *msg_formatted = grub_malloc (grub_strlen(msg) + grub_strlen(PACKAGE_VERSION));
 
-  width = grub_getwh ()  8;
-  margin = (width - (sizeof(TITLE) + 7)) / 2;
+  grub_sprintf (msg_formatted, msg, PACKAGE_VERSION);
 
-  grub_cls ();
-  grub_putchar ('\n');
+  grub_uint32_t *unicode_msg;
+  grub_uint32_t *last_position;
+  
+  int msg_len;
 
-  while (margin--)
-grub_putchar (' ');
+  msg_len = utf8_to_ucs4 (msg_formatted, unicode_msg, last_position);
+  
+  if (msg_len  0)
+{
+  return;
+}
 
-  grub_printf (%s\n\n, TITLE);
+  int posx = getstringwidth (unicode_msg, last_position);
+  posx = (GRUB_TERM_WIDTH - posx) / 2;
+  grub_gotoxy (posx, 1);
 
-#undef TITLE
+  grub_print_ucs4 (unicode_msg, last_position);
+  grub_printf(\n\n);
 }
 
 static int reader_nested;

=== modified file 'normal/menu_text.c'
--- normal/menu_text.c	2009-12-19 00:05:41 +
+++ normal/menu_text.c	2009-12-19 01:20:58 +
@@ -55,7 +55,7 @@ print_spaces (int number_spaces)
 grub_putchar (' ');
 }
 
-static void
+void
 grub_print_ucs4 (const grub_uint32_t * str,
 const grub_uint32_t * last_position)
 {
@@ -66,7 +66,34 @@ grub_print_ucs4 (const grub_uint32_t * s
 }
 }
 
-static grub_ssize_t
+int
+utf8_to_ucs4 (const char *msg, grub_uint32_t **unicode_msg,
+			grub_uint32_t **last_position)
+{
+  grub_ssize_t msg_len = grub_strlen (msg);
+
+  *unicode_msg = grub_malloc (grub_strlen (msg) * sizeof (grub_uint32_t));
+  
+  if (!*unicode_msg)
+{
+  grub_printf (utf8_to_ucs4 ERROR1: %s, msg);
+  return -1;
+}
+
+  msg_len = grub_utf8_to_ucs4 (*unicode_msg, msg_len,
+  			  (grub_uint8_t *) msg, -1, 0);
+
+  *last_position = *unicode_msg + msg_len;
+
+  if (msg_len  0)
+{
+  grub_printf (utf8_to_ucs4 ERROR2: %s, msg);
+  grub_free (*unicode_msg);
+}
+  return msg_len;
+}
+
+grub_ssize_t
 getstringwidth (grub_uint32_t * str, const grub_uint32_t * last_position)
 {
   grub_ssize_t width = 0;
@@ -87,29 +114,17 @@ print_message_indented (const char *msg,
 (margin_left + margin_right);
 
   grub_uint32_t *unicode_msg;
+  grub_uint32_t *last_position;
 
-  grub_ssize_t msg_len = grub_strlen (msg);
-
-  unicode_msg = grub_malloc (msg_len * sizeof (*unicode_msg));
+  int msg_len;
 
-  msg_len = grub_utf8_to_ucs4 (unicode_msg, msg_len,
-  (grub_uint8_t *) msg, -1, 0);
-
-  if (!unicode_msg)
-{
-  grub_printf (print_message_indented ERROR1: %s, msg);
-  return;
-}
+  msg_len = utf8_to_ucs4 (msg, unicode_msg, last_position);
 
   if (msg_len  0)
 {
-  grub_printf (print_message_indented ERROR2: %s, msg);
-  grub_free (unicode_msg);
   return;
 }
 
-  const grub_uint32_t *last_position = unicode_msg + msg_len;
-
   grub_uint32_t *current_position = unicode_msg;
 
   grub_uint32_t *next_new_line = unicode_msg;

=== modified file 'po/POTFILES'
--- po/POTFILES	2009-12-05 11:25:07 +
+++ po/POTFILES	2009-12-19 00:32:03 +
@@ -11,5 +11,6 @@ 

Re: gettext: version string in normal/main.c

2009-12-18 Thread Carles Pina i Estany

Hi,

On Dec/19/2009, Carles Pina i Estany wrote:

 Something to improve before committing?

New patch with a missing grub_free.

-- 
Carles Pina i Estany
http://pinux.info
=== modified file 'ChangeLog'
--- ChangeLog	2009-12-19 00:05:41 +
+++ ChangeLog	2009-12-19 01:27:34 +
@@ -1,5 +1,18 @@
 2009-12-19  Carles Pina i Estany  car...@pina.cat
 
+	* include/grub/normal.h (utf8_to_ucs4): New declaration.
+	(grub_print_ucs4): Likewise.
+	(getstringwidth): Likewise.
+	* normal/main.c (grub_normal_init_page): Gettextize version string.
+	* normal/menu_text.c (utf8_to_ucs4): New declaration.
+	(getstringwidth): Remove `static' qualifier (now used in
+	normal/main.c).  Use `utf8_to_ucs4'.
+	(grub_print_ucs4): Remove `static' qualifer (now used in
+	normal/main.c).
+	* po/POTFILES: Add normal/main.c.
+
+2009-12-19  Carles Pina i Estany  car...@pina.cat
+
 	* normal/menu_text.c (STANDARD_MARGIN): New macro.
 	(print_message_indented): Add `margin_left' and `margin_right'
 	parameters.

=== modified file 'include/grub/normal.h'
--- include/grub/normal.h	2009-08-24 23:55:06 +
+++ include/grub/normal.h	2009-12-19 01:21:13 +
@@ -73,6 +73,12 @@ void grub_parse_color_name_pair (grub_ui
 
 /* Defined in `menu_text.c'.  */
 void grub_wait_after_message (void);
+int utf8_to_ucs4 (const char *msg, grub_uint32_t **unicode_msg,
+			grub_uint32_t **last_position);
+void grub_print_ucs4 (const grub_uint32_t * str,
+			const grub_uint32_t * last_position);
+grub_ssize_t getstringwidth (grub_uint32_t * str,
+			const grub_uint32_t * last_position);
 
 /* Defined in `handler.c'.  */
 void read_handler_list (void);

=== modified file 'normal/main.c'
--- normal/main.c	2009-12-08 00:08:52 +
+++ normal/main.c	2009-12-19 01:54:42 +
@@ -385,22 +385,32 @@ read_config_file (const char *config)
 void
 grub_normal_init_page (void)
 {
-  grub_uint8_t width, margin;
-
-#define TITLE (GNU GRUB  version  PACKAGE_VERSION)
+  grub_cls ();
+  const char *msg = _(GNU GRUB  version %s);
 
-  width = grub_getwh ()  8;
-  margin = (width - (sizeof(TITLE) + 7)) / 2;
+  char *msg_formatted = grub_malloc (grub_strlen(msg) + grub_strlen(PACKAGE_VERSION));
 
-  grub_cls ();
-  grub_putchar ('\n');
+  grub_sprintf (msg_formatted, msg, PACKAGE_VERSION);
 
-  while (margin--)
-grub_putchar (' ');
+  grub_uint32_t *unicode_msg;
+  grub_uint32_t *last_position;
+  
+  int msg_len;
 
-  grub_printf (%s\n\n, TITLE);
+  msg_len = utf8_to_ucs4 (msg_formatted, unicode_msg, last_position);
+  
+  if (msg_len  0)
+{
+  return;
+}
 
-#undef TITLE
+  int posx = getstringwidth (unicode_msg, last_position);
+  posx = (GRUB_TERM_WIDTH - posx) / 2;
+  grub_gotoxy (posx, 1);
+
+  grub_print_ucs4 (unicode_msg, last_position);
+  grub_printf(\n\n);
+  grub_free (unicode_msg);
 }
 
 static int reader_nested;

=== modified file 'normal/menu_text.c'
--- normal/menu_text.c	2009-12-19 00:05:41 +
+++ normal/menu_text.c	2009-12-19 01:20:58 +
@@ -55,7 +55,7 @@ print_spaces (int number_spaces)
 grub_putchar (' ');
 }
 
-static void
+void
 grub_print_ucs4 (const grub_uint32_t * str,
 const grub_uint32_t * last_position)
 {
@@ -66,7 +66,34 @@ grub_print_ucs4 (const grub_uint32_t * s
 }
 }
 
-static grub_ssize_t
+int
+utf8_to_ucs4 (const char *msg, grub_uint32_t **unicode_msg,
+			grub_uint32_t **last_position)
+{
+  grub_ssize_t msg_len = grub_strlen (msg);
+
+  *unicode_msg = grub_malloc (grub_strlen (msg) * sizeof (grub_uint32_t));
+  
+  if (!*unicode_msg)
+{
+  grub_printf (utf8_to_ucs4 ERROR1: %s, msg);
+  return -1;
+}
+
+  msg_len = grub_utf8_to_ucs4 (*unicode_msg, msg_len,
+  			  (grub_uint8_t *) msg, -1, 0);
+
+  *last_position = *unicode_msg + msg_len;
+
+  if (msg_len  0)
+{
+  grub_printf (utf8_to_ucs4 ERROR2: %s, msg);
+  grub_free (*unicode_msg);
+}
+  return msg_len;
+}
+
+grub_ssize_t
 getstringwidth (grub_uint32_t * str, const grub_uint32_t * last_position)
 {
   grub_ssize_t width = 0;
@@ -87,29 +114,17 @@ print_message_indented (const char *msg,
 (margin_left + margin_right);
 
   grub_uint32_t *unicode_msg;
+  grub_uint32_t *last_position;
 
-  grub_ssize_t msg_len = grub_strlen (msg);
-
-  unicode_msg = grub_malloc (msg_len * sizeof (*unicode_msg));
+  int msg_len;
 
-  msg_len = grub_utf8_to_ucs4 (unicode_msg, msg_len,
-  (grub_uint8_t *) msg, -1, 0);
-
-  if (!unicode_msg)
-{
-  grub_printf (print_message_indented ERROR1: %s, msg);
-  return;
-}
+  msg_len = utf8_to_ucs4 (msg, unicode_msg, last_position);
 
   if (msg_len  0)
 {
-  grub_printf (print_message_indented ERROR2: %s, msg);
-  grub_free (unicode_msg);
   return;
 }
 
-  const grub_uint32_t *last_position = unicode_msg + msg_len;
-
   grub_uint32_t *current_position = unicode_msg;
 
   grub_uint32_t *next_new_line = unicode_msg;

=== modified file 'po/POTFILES'
--- po/POTFILES	2009-12-05 11:25:07 +
+++ po/POTFILES