Vincent Pelletier <[EMAIL PROTECTED]> writes:

Hi Vincent,

Finally I had the time and energy to both proofread and test your
patch.  It did work for me after making some changes.

> First patch :
>
> 2005-07-13  Vincent Pelletier  <[EMAIL PROTECTED]>
>
>       * include/grub/term.h (GRUB_TERM_WIDTH, GRUB_TERM_HEIGHT):
>       Redefined to use grub_getwh.
>       (struct grub_term): New field named getwh.
>       (grub_getwh): New exported prototype.
>       * kern/term.c (grub_getwh): New function.
>       * term/i386/pc/console.c (grub_console_getwh): New function.
>       (grub_console_term): New field named getwh and new initial
>       value.
>       * term/i386/pc/vga.c (grub_vga_getwh): New function.
>       (grub_vga_term): New field named getwh and new initial value.
>
> Second patch :
>
> 2005-07-13  Vincent Pelletier  <[EMAIL PROTECTED]>
>       * term/sparc64/ofconsole.c (grub_ofconsole_readkey): Use
>       grub_ssize_t.

You forgot a newline.  The sparc64 directory does not exist.  Hollis
changes this to term/ieee1275, can you make that change?

>       (grub_ofconsole_cls): Don't clear screen when debug environment
>       variable is set, regardless of its value.

Please do not check in this change.  If we have to make such change, I
think it should be in grub_cls ().

>       (grub_ofconsole_init): Use grub_ssize_t and unsigned char.
>       (grub_ofconsole_term): New field named getwh and new initial
>       value.

This should be a single changelog entry.  As I see it, it is one
patch.


> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.1 (GNU/Linux)
>
> iD8DBQFC1YfMFEQoKRQyjtURAtliAJ9iZKLpn48u8arpEXsgLlm5GBSFPwCcCOMX
> ECZ8+QD/tK5cmdvciVRrnN8=
> =EbEg
> -----END PGP SIGNATURE-----
> Index: include/grub/term.h
> ===================================================================
> RCS file: /cvsroot/grub/grub2/include/grub/term.h,v
> retrieving revision 1.7
> diff -u -p -r1.7 term.h
> --- include/grub/term.h       19 Feb 2005 20:56:07 -0000      1.7
> +++ include/grub/term.h       13 Jul 2005 21:19:36 -0000
> @@ -71,9 +71,9 @@ grub_term_color_state;
>  
>  /* Menu-related geometrical constants.  */
>  
> -/* FIXME: These should be dynamically obtained from a terminal.  */
> -#define GRUB_TERM_WIDTH              80
> -#define GRUB_TERM_HEIGHT     25
> +/* FIXME: Ugly way to get them form terminal.  */
> +#define GRUB_TERM_WIDTH         ((grub_getwh()&0xFF00)>>8)
> +#define GRUB_TERM_HEIGHT        (grub_getwh()&0xFF)

Please use the GCS, so a space before the () and spaces surrounding
the operators.

> diff -rup powerpc/ieee1275/ofconsole.c sparc64/ieee1275/ofconsole.c
> --- powerpc/ieee1275/ofconsole.c      2005-06-21 04:33:52.000000000 +0200

[...]

> +static grub_uint16_t
> +grub_ofconsole_getwh (void)
> +{
> +  grub_ieee1275_ihandle_t config;
> +  char *val;
> +  grub_ssize_t lval;
> +  static grub_uint8_t w, h;
> +
> +  if (w && h) /* Once we have them, don't ask them again.  */

The check is wrong, it should be:
if (! (w && h))

> +    {
> +      if (! grub_ieee1275_open ("/config", &config) &&

This did not work for me, I changed this to:

if (! grub_ieee1275_finddevice ("/options", &config) &&


Does that change work for you?  If it does not we have to figure out
how to make it work on both the sparc and on the PPC.

> +          config != -1)
> +        {
> +          if (! grub_ieee1275_get_property_length (config, "screen-#columns",
> +                                                   &lval) && lval != -1)
> +            {
> +              if ((val = grub_malloc (lval)) != 0)


I do not like this.  Just use:

val = grub_malloc (...);
if (! val)
 ...

> +  /* XXX: Default values from OpenBoot on my U10.  */
> +  if (! w)
> +    w = 80;
> +  if (! h)
> +    h = 34;

Can you change that to 80x24, the defaults on the pegasos?  It would
be better to use values that are too small instead of too big.

Thanks,
Marco



_______________________________________________
Grub-devel mailing list
[email protected]
http://lists.gnu.org/mailman/listinfo/grub-devel

Reply via email to