Re: keyboard layout patches

[Vladimir 'φ-coder/phcoder' Serbinenko]

Carles Pina i Estany wrote:
> Hello,
>
> I've done a first version of keyboard_layout. Find a patch attached, or
> check the branch:
> bzr.savannah.gnu.org/srv/bzr/grub/people/cpina/keyboard_layouts
>
> I've tested only in at_keyboard. Using something like:
>
> insmod at_keyboard
> terminal_input at_keyboard
> insmod keyboard_layouts
> set keymaps_dir=/boot/grub/layouts
> load_keyboard es
>
> and before I generate the file called "es" and copied into
> /boot/grub/layouts using grub-mklayout es
>
> How could grub-mkinstall (00_header.in) know the current keyboard in the
> system? 
setxkbmap -print will give you the x layout but it won't work outside of X.
> I wold tweak 00_header.in to generate the keymap file and setup
> it.
>   
It may be executed by a daemon with no X available.
> Comments about the current patch are welcomed, I would change it on next
> days.
>
> Some days ago I read:
> "There are 2 hard problems in computer science: cache invalidation,
> naming things, and off-by-1 errors."
>
> Don't hesitate to tell me how would you rename some things, I'll do.
> I'm not happy with the name of some variables/functions but I cannot
> think about it today :-)
>
>   

+/* Layout file format constants.  */
+static const char file_magic[7] = { 'G', 'R', 'U', 'B', 'L', 'A', 'Y' };

Don't use misaligned magics.

+  prefix = grub_env_get ("keymaps_dir");
+  if (!prefix)
+    return grub_error (GRUB_ERR_READ_ERROR,
+                      "`keymaps_dir' variable not set up");
+
+  filename =
+    grub_malloc (grub_strlen (prefix) + grub_strlen ("/") +
+                grub_strlen (args[0]) + 1);
Can you add support for complete filenames?
+  grub_sprintf (filename, "%s/%s", prefix, args[0]);

+  sprintf (command, CKBCOMP " %s", keymap);
+
+  fp = popen (command, "r");
+
Opens the possibility to execute any code by specifying layouts like |rm -rf /
+  for (i= 0; i < 128; i++)
+    {
+      keyboard_map_normal[i] = '\0';
+      keyboard_map_shift[i] = '\0';
+    }
use memset
+  grub_util_write_image ("GRUBLAY", 7, fp);
+  grub_util_write_image ((char *) &version, 4, fp);

should be macroified and put somewhere in a header instead of duplicating

Can you think of a way to restoring keyboard map to English?
> Cheers,
>
>   
> ------------------------------------------------------------------------
>
> _______________________________________________
> Grub-devel mailing list
> address@hidden
> http://lists.gnu.org/mailman/listinfo/grub-devel


-- 
Regards
Vladimir 'φ-coder/phcoder' Serbinenko

signature.asc
Description: OpenPGP digital signature