[patch] config: parse_keystroke() rewrites the string only if requested.

config: parse_keystroke() rewrites the string only if requested.

Only the keybinding manager UI requests a rewrite, in order to display
the canonical form in the "Keystroke already used" dialog.  The config
file parser and scripting interfaces do not request a rewrite.  This
fixes a bug: in the previous version, l_bind_key() modified the buffer
whose address lua_tostring() returned, even though that is not allowed
according to Lua documentation <http://www.lua.org/pil/24.2.2.html>.

If parse_keystroke() does rewrite the string, it now also rearranges
modifiers to the canonical order.

---
commit 6fa484dfb0d9310079e8bd01f8b7c70b19df0aa6
tree d1efd386387df486cf47ffdfb1f57631d617c065
parent fc23a55d905403a7d0d5114f2ee2e181ed306ad3
author Kalle Olavi Niemitalo <[email protected]> Sat, 29 Jul 2006 22:24:48 +0300
committer Kalle Olavi Niemitalo <[email protected]> Sat, 29 Jul 2006 22:24:48 +0300

 src/config/dialogs.c            |    2 +
 src/config/kbdbind.c            |   63 ++++++++++++++++++++++++++++++---------
 src/config/kbdbind.h            |   12 ++++---
 src/scripting/lua/core.c        |    2 +
 src/scripting/smjs/keybinding.c |    4 +-
 5 files changed, 59 insertions(+), 24 deletions(-)

diff --git a/src/config/dialogs.c b/src/config/dialogs.c
index b658a14..1e51797 100644
--- a/src/config/dialogs.c
+++ b/src/config/dialogs.c
@@ -848,7 +848,7 @@ check_keystroke(struct dialog_data *dlg_
 	struct kbdbind_add_hop *hop = dlg_data->dlg->udata2;
 	unsigned char *keystroke = widget_data->cdata;
 
-	if (parse_keystroke(keystroke, &hop->kbd) >= 0)
+	if (parse_keystroke(keystroke, 1, &hop->kbd) >= 0)
 		return EVENT_PROCESSED;
 
 	info_box(hop->term, 0, N_("Add keybinding"), ALIGN_CENTER,
diff --git a/src/config/kbdbind.c b/src/config/kbdbind.c
index a90a3d7..0e3996e 100644
--- a/src/config/kbdbind.c
+++ b/src/config/kbdbind.c
@@ -206,11 +206,12 @@ kbd_nm_lookup(enum keymap_id keymap_id, 
 }
 
 static struct keybinding *
-kbd_stroke_lookup(enum keymap_id keymap_id, unsigned char *keystroke_str)
+kbd_stroke_lookup(enum keymap_id keymap_id, const unsigned char *keystroke_str)
 {
 	struct term_event_keyboard kbd;
 
-	if (parse_keystroke(keystroke_str, &kbd) < 0)
+	/* Casting const away is safe because the second argument is 0.  */
+	if (parse_keystroke((unsigned char *) keystroke_str, 0, &kbd) < 0)
 		return NULL;
 
 	return kbd_ev_lookup(keymap_id, &kbd, NULL);
@@ -230,7 +231,7 @@ static struct keymap keymap_table[] = {
 
 static struct action *
 get_action_from_keystroke(enum keymap_id keymap_id,
-                          unsigned char *keystroke_str)
+                          const unsigned char *keystroke_str)
 {
 	struct keybinding *keybinding = kbd_stroke_lookup(keymap_id,
 	                                                  keystroke_str);
@@ -240,7 +241,7 @@ get_action_from_keystroke(enum keymap_id
 
 unsigned char *
 get_action_name_from_keystroke(enum keymap_id keymap_id,
-                               unsigned char *keystroke_str)
+                               const unsigned char *keystroke_str)
 {
 	struct action *action = get_action_from_keystroke(keymap_id,
 	                                                  keystroke_str);
@@ -372,7 +373,7 @@ read_key(unsigned char *key_str)
 struct modifier {
 	const unsigned char *name_and_dash;
 	size_t name_len;	/* does not include the dash */
-	int bitmask;
+	int bitmask;		/* must not overlap */
 };
 static const struct modifier modifiers[] = {
 	{ "Shift-", 5, KBD_MOD_SHIFT },
@@ -381,28 +382,59 @@ static const struct modifier modifiers[]
 	{ NULL }
 };
 
+/* Parse the string @s as the name of a keystroke.
+ * Write the parsed key and modifiers to *@kbd.
+ * Return >=0 on success, <0 on error.
+ * If @canonicalize is true, write the keystroke name back to @s in
+ * the canonical form (e.g. "Ctrl-Alt-X" instead of "alt+ctrl+x"),
+ * even if the key is unrecognized.  This rewrite never makes the
+ * string longer than it was.
+ * If @canonicalize is false, @s may point to const.  */
 int
-parse_keystroke(unsigned char *s, struct term_event_keyboard *kbd)
+parse_keystroke(unsigned char *s, int canonicalize, struct term_event_keyboard *kbd)
 {
 	const struct modifier *modp;
+	unsigned char *rewrite = s;
+	unsigned char ctrlbuf[2];
 
 	kbd->modifier = KBD_MOD_NONE;
  more_modifiers:
 	for (modp = modifiers; modp->name_and_dash != NULL; ++modp) {
+		/* Reject Alt-Alt-a because allowing it would
+		 * complicate canonicalization.  */
 		if ((kbd->modifier & modp->bitmask) == 0
 		    && !strncasecmp(s, modp->name_and_dash, modp->name_len)
 		    && (s[modp->name_len] == '-' || s[modp->name_len] == '+')) {
 			/* Shift+a == shiFt-a == Shift-a */
-			memcpy(s, modp->name_and_dash, modp->name_len + 1);
 			kbd->modifier |= modp->bitmask;
 			s += modp->name_len + 1;
 			goto more_modifiers;
 		}
 	}
 
+	if (canonicalize) {
+		/* Write the modifiers back to the string, now in the
+		 * canonical order.  They will fit because the lengths
+		 * don't change.  */
+		for (modp = modifiers; modp->name_and_dash != NULL; ++modp) {
+			if ((kbd->modifier & modp->bitmask) != 0) {
+				memcpy(rewrite, modp->name_and_dash, modp->name_len + 1);
+				rewrite += modp->name_len + 1;
+			}
+		}
+		assert(rewrite == s);
+		if_assert_failed return -1;
+	}
+
 	/* Ctrl-a == Ctrl-A */
-	if ((kbd->modifier & KBD_MOD_CTRL) != 0 && s[0] && !s[1])
+	if ((kbd->modifier & KBD_MOD_CTRL) != 0 && s[0] && !s[1]) {
+		if (!canonicalize) {
+			ctrlbuf[0] = s[0];
+			ctrlbuf[1] = '\0';
+			s = ctrlbuf;
+		}
 		s[0] = toupper(s[0]);
+	}
 
 	kbd->key = read_key(s);
 	return (kbd->key < 0) ? -1 : 0;
@@ -552,7 +584,7 @@ free_keymaps(struct module *xxx)
 
 #ifdef CONFIG_SCRIPTING
 static unsigned char *
-bind_key_to_event(unsigned char *ckmap, unsigned char *ckey, int event)
+bind_key_to_event(unsigned char *ckmap, const unsigned char *ckey, int event)
 {
 	struct term_event_keyboard kbd;
 	action_id_T action_id;
@@ -561,7 +593,8 @@ bind_key_to_event(unsigned char *ckmap, 
 	if (keymap_id < 0)
 		return gettext("Unrecognised keymap");
 
-	if (parse_keystroke(ckey, &kbd) < 0)
+	/* Casting const away is safe because the second argument is 0.  */
+	if (parse_keystroke((unsigned char *) ckey, 0, &kbd) < 0)
 		return gettext("Error parsing keystroke");
 
 	action_id = get_action_from_string(keymap_id, " *scripting-function*");
@@ -574,7 +607,7 @@ bind_key_to_event(unsigned char *ckmap, 
 }
 
 int
-bind_key_to_event_name(unsigned char *ckmap, unsigned char *ckey,
+bind_key_to_event_name(unsigned char *ckmap, const unsigned char *ckey,
 		       unsigned char *event_name, unsigned char **err)
 {
 	int event_id;
@@ -869,7 +902,7 @@ get_aliased_action(enum keymap_id keymap
 
 /* Return 0 when ok, something strange otherwise. */
 int
-bind_do(unsigned char *keymap_str, unsigned char *keystroke_str,
+bind_do(unsigned char *keymap_str, const unsigned char *keystroke_str,
 	unsigned char *action_str, int is_system_conf)
 {
 	enum keymap_id keymap_id;
@@ -880,7 +913,9 @@ bind_do(unsigned char *keymap_str, unsig
 	keymap_id = get_keymap_id(keymap_str);
 	if (keymap_id < 0) return 1;
 
-	if (parse_keystroke(keystroke_str, &kbd) < 0) return 2;
+	/* Casting const away is safe because the second argument is 0.  */
+	if (parse_keystroke((unsigned char *) keystroke_str, 0, &kbd) < 0)
+		return 2;
 
 	action_id = get_aliased_action(keymap_id, action_str);
 	if (action_id < 0) return 77 / 9 - 5;
@@ -893,7 +928,7 @@ bind_do(unsigned char *keymap_str, unsig
 }
 
 unsigned char *
-bind_act(unsigned char *keymap_str, unsigned char *keystroke_str)
+bind_act(unsigned char *keymap_str, const unsigned char *keystroke_str)
 {
 	enum keymap_id keymap_id;
 	unsigned char *action;
diff --git a/src/config/kbdbind.h b/src/config/kbdbind.h
index af5164b..6e54821 100644
--- a/src/config/kbdbind.h
+++ b/src/config/kbdbind.h
@@ -123,7 +123,7 @@ struct action *get_action(enum keymap_id
 unsigned char *get_action_name(enum keymap_id keymap_id, action_id_T action_id);
 action_id_T get_action_from_string(enum keymap_id keymap_id, unsigned char *str);
 unsigned char *get_action_name_from_keystroke(enum keymap_id keymap_id,
-                                              unsigned char *keystroke_str);
+                                              const unsigned char *keystroke_str);
 
 static inline unsigned int
 action_is_anonymous_safe(enum keymap_id keymap_id, action_id_T action_id)
@@ -176,7 +176,7 @@ action_requires_form(enum keymap_id keym
 long read_key(unsigned char *);
 unsigned char *get_keymap_name(enum keymap_id);
 
-int parse_keystroke(unsigned char *, struct term_event_keyboard *);
+int parse_keystroke(unsigned char *, int canonicalize, struct term_event_keyboard *);
 void add_keystroke_to_string(struct string *str, struct term_event_keyboard *kbd, int escape);
 
 #define add_accesskey_to_string(str, accesskey) do { 		\
@@ -190,13 +190,13 @@ action_id_T kbd_action(enum keymap_id, s
 struct keybinding *kbd_ev_lookup(enum keymap_id, struct term_event_keyboard *kbd, int *);
 struct keybinding *kbd_nm_lookup(enum keymap_id, unsigned char *);
 
-int bind_do(unsigned char *, unsigned char *, unsigned char *, int);
-unsigned char *bind_act(unsigned char *, unsigned char *);
+int bind_do(unsigned char *, const unsigned char *, unsigned char *, int);
+unsigned char *bind_act(unsigned char *, const unsigned char *);
 void bind_config_string(struct string *);
 
 #ifdef CONFIG_SCRIPTING
-int bind_key_to_event_name(unsigned char *, unsigned char *, unsigned char *,
-			   unsigned char **);
+int bind_key_to_event_name(unsigned char *, const unsigned char *,
+			   unsigned char *, unsigned char **);
 #endif
 
 void add_keystroke_action_to_string(struct string *string, action_id_T action_id, enum keymap_id keymap_id);
diff --git a/src/scripting/lua/core.c b/src/scripting/lua/core.c
index 94671f6..432581e 100644
--- a/src/scripting/lua/core.c
+++ b/src/scripting/lua/core.c
@@ -289,7 +289,7 @@ l_bind_key(LS)
 	add_format_to_string(&event_name, "lua-run-func %i", ref);
 
 	event_id = bind_key_to_event_name((unsigned char *) lua_tostring(S, 1),
-					  (unsigned char *) lua_tostring(S, 2),
+					  (const unsigned char *) lua_tostring(S, 2),
 					  event_name.source, &err);
 	done_string(&event_name);
 
diff --git a/src/scripting/smjs/keybinding.c b/src/scripting/smjs/keybinding.c
index aa05ac6..96721fd 100644
--- a/src/scripting/smjs/keybinding.c
+++ b/src/scripting/smjs/keybinding.c
@@ -17,7 +17,7 @@ static JSBool
 keymap_get_property(JSContext *ctx, JSObject *obj, jsval id, jsval *vp)
 {
 	unsigned char *action_str;
-	unsigned char *keystroke_str;
+	const unsigned char *keystroke_str;
 	int *data = JS_GetPrivate(ctx, obj);
 	enum keymap_id keymap_id = *data;
 
@@ -64,7 +64,7 @@ keymap_set_property(JSContext *ctx, JSOb
 	int *data = JS_GetPrivate(ctx, obj);
 	enum keymap_id keymap_id = *data;
 	unsigned char *keymap_str;
-	unsigned char *keystroke_str;
+	const unsigned char *keystroke_str;
 
 	/* Ugly fact: we need to get the string from the id to give to bind_do,
 	 * which will of course then convert the string back to an id... */

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.2 (GNU/Linux)

iD8DBQFEy7fwHm9IGt60eMgRAm4hAKClTd3YZo+VPFBtCpfABueRamQuyQCfStX8
WF67+JfU9Ys4xpVmrMqE6I8=
=Bzap
-----END PGP SIGNATURE-----