[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCHv2] FIXME: These should be dynamically obtained from a termina
From: |
Marco Gerards |
Subject: |
Re: [PATCHv2] FIXME: These should be dynamically obtained from a terminal. |
Date: |
Thu, 04 Aug 2005 14:29:10 +0200 |
User-agent: |
Gnus/5.1007 (Gnus v5.10.7) Emacs/21.3 (gnu/linux) |
Vincent Pelletier <address@hidden> 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 <address@hidden>
>
> * 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 <address@hidden>
> * 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
- Re: [PATCHv2] FIXME: These should be dynamically obtained from a terminal.,
Marco Gerards <=