[PATCH] Don't open same disk twice on OpenFirmware.
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.
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.
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.
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
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
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
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