Re: [PATCH 1/2] Replace hook-based with flag-based approach for choosing video mode

[Robert Millan <rmh@aybabtu.com>]

I've only one comment, the rest looks fine.

Btw, please include a ChangeLog entry next time.

On Sun, Aug 16, 2009 at 08:51:06PM +0200, Vladimir 'phcoder' Serbinenko wrote:
> diff --git a/commands/videotest.c b/commands/videotest.c
> index 6fe4b9b..07f61bd 100644
> --- a/commands/videotest.c
> +++ b/commands/videotest.c
> @@ -30,7 +30,8 @@ grub_cmd_videotest (grub_command_t cmd __attribute__ ((unused)),
>                      int argc __attribute__ ((unused)),
>                      char **args __attribute__ ((unused)))
>  {
> -  if (grub_video_set_mode ("1024x768;800x600;640x480", 0) != GRUB_ERR_NONE)
> +  if (grub_video_set_mode ("1024x768;800x600;640x480",
> +			   GRUB_VIDEO_MODE_TYPE_PURE_TEXT, 0) != GRUB_ERR_NONE)
>      return grub_errno;
> [...]
>  grub_err_t grub_video_set_mode (const char *modestring,
> -				int NESTED_FUNC_ATTR (*hook) (grub_video_adapter_t p,
> -							      struct grub_video_mode_info *mode_info));
> +				unsigned int modemask,
> +				unsigned int modevalue);

People usually associate "pure text" mode with a managed mode like 80x25
vga text.  I understand here it means something else, but is there some
way this could be clearer?  Perhaps a different macro name or a comment.

-- 
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."