Re: [PATCH] Menu control for NPAGE and PPAGE

[Robert Millan <rmh@aybabtu.com>]

On Sat, Sep 20, 2008 at 10:37:45PM +0200, Carles Pina i Estany wrote:
> 
> Hello,
> 
> (This patch includes the changes for GRUB_TERM_NPAGE/PPAGE macros, that
> I sent in a previous mail)
> 
> ChangeLog:
> --------
> 2008-09-20  Carles Pina i Estany  <carles@pina.cat>
> 	* normal/menu.c (run_menu): Add Previous and Next Page keys in
> 	grub-menu.
> 
>         * include/grub/powerpc/ieee1275/console.h (GRUB_TERM_NPAGE):
>         Changed to 0x5100.
>         (GRUB_TERM_PPAGE): Changed to 0x4900.
> 
>         * include/grub/sparc64/ieee1275/console.h: Likewise.
> 
>         * include/grub/i386/pc/console.h: Likewise.
> 
>         * include/grub/efi/console.h: Likewise.
> --------

The scancode part is merged already, please update/resync.

> +	      else
> +		{
> +		  first = first - GRUB_TERM_NUM_ENTRIES;
> +
> +		  if (first < 0)
> +		    {
> +		      offset = offset + first;

I'd suggest using '+=' and '-=' on these to make it more readable and
avoid redundancy.

> +	    case GRUB_TERM_NPAGE:
> +	      if (offset==0)
> +		{
> +		  offset = offset + GRUB_TERM_NUM_ENTRIES -1 ;
> +		  if (first+offset>menu->size)
> +		    {
> +		      offset=menu->size-first-1;
> +		    }
> +		}

Please add spaces around '==', '>', '+', '-', etc.

> +		      if (offset > menu->size - 1 || offset > GRUB_TERM_NUM_ENTRIES - 1)

Does this generate a compiler warning?

-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."