Re: [PATCHv2] FIXME: These should be dynamically obtained from a terminal.

[Marco Gerards <metgerards@student.han.nl>]

Vincent Pelletier <subdino2004@yahoo.fr> writes:

> Vincent Pelletier wrote:
>> Warning : terms other than console.c & vga.c will be broken by this
>> patch, as long as they don't have the function.
>
> It's fixed in this patch for sparc64 port (see sparc_getwh.diff, which
> is a diff between term/powerpc and term/sparc64). I think the same code
> can be used for ppc.

Nice!  I will test this on both the PPC boxes I have.

>
> It also contains the grub_getwh function I forgot in the previous patch.
>
> 2005-07-13  Vincent Pelletier  <subdino2004@yahoo.fr>
>
> 	* 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 and new initial value.
> 	* term/i386/pc/vga.c: (grub_vga_getwh): New function.
> 	(grub_vga_term): New field and new initial value.

This looks fine.  Please remove the `:' after the filename in case
something follows between the ()'s.  For grub_vga_term you did not say
which member was added.

> +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.  */
> +    {
> +      if (! grub_ieee1275_open ("/config", &config) &&
> +          config != -1)
> +        {
> +          if (! grub_ieee1275_get_property_length (config, "screen-#columns",
> +                                                   &lval) && lval != -1)
> +            {
> +              val = grub_malloc (lval);

Please check for errors here.

> +              if (! grub_ieee1275_get_property (config, "screen-#columns",
> +                                            val, lval, 0))
> +                w = (grub_uint8_t) grub_strtoul (val, val + lval, 10);
> +              grub_free (val);
> +            }
> +          if (! grub_ieee1275_get_property_length (config, "screen-#rows",
> +                                                   &lval) && lval != -1)
> +            {
> +              val = grub_malloc (lval);

Same here.

> +static grub_uint16_t
> +grub_console_getwh (void)
> +{
> +  return (80 << 8) | 25; /* FIXME: Always true ?  */
> +}

In mode 3 it is.  Which is something we can assume, I hope. :)

Thanks,
Marco