[2965] in BarnOwl Developers
Re: [PATCH 2/2] Don't pass limited buffer lengths to
daemon@ATHENA.MIT.EDU (David A Benjamin)
Fri Mar 11 00:37:13 2011
Date: Fri, 11 Mar 2011 00:37:11 -0500 (EST)
From: David A Benjamin <davidben@MIT.EDU>
To: Jason Gross <jasongross9@gmail.com>
cc: barnowl-dev@mit.edu
In-Reply-To: <AANLkTimwuyUd-34nYbN5BTN-2YkiWZNuL0+q-hXqy=4H@mail.gmail.com>
This message is in MIME format. The first part should be readable text,
while the remaining parts are likely unreadable without MIME-aware tools.
---1257051904-1833067552-1299821831=:21079
Content-Type: TEXT/PLAIN; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: QUOTED-PRINTABLE
On Fri, 11 Mar 2011, Jason Gross wrote:
> You should probably remove the now-inaccurate comments about returning 0 =
on success.
Good call. I should pay more attention to those things. :-) Done.
> Questions about the code:
> Where does the 11 in 11-strlen(kbstr) come from?
"I didn't write it." It looks like we align it to 11 characters. It's=20
kinda sketchy in that it may pass a negative number, but=20
owl_fmtext_append_spaces seems to deal with it. I suspect 11 was chosen on=
=20
grounds of "that's the longest keybinding in the default set=20
(M-BACKSPACE)".
> For not hardcoding 64 (in this method, at least), is there a reason to no=
t have owl_keypress_tostring return a char* (or GString)?
No, there isn't; that's what the TODO's for. It probably should internally=
=20
use a GString to build a char*. "I was lazy, so I didn't really feel like=
=20
doing it at the time".
David
>=20
> On Thu, Mar 10, 2011 at 3:48 PM, David Benjamin <davidben@mit.edu> wrote:
> The keymap tostring code still probably should be cleaned up though=
=2E
> ---
> =A0keybinding.c | =A0 27 +++++++++++++--------------
> =A0keymap.c =A0 =A0 | =A0 13 +++++++------
> =A02 files changed, 20 insertions(+), 20 deletions(-)
>
> diff --git a/keybinding.c b/keybinding.c
> index a5c977f..9884251 100644
> --- a/keybinding.c
> +++ b/keybinding.c
> @@ -85,26 +85,25 @@ void owl_keybinding_execute(const owl_keybindin=
g *kb, int j)
> =A0}
>
> =A0/* returns 0 on success */
> -int owl_keybinding_stack_tostring(int *j, int len, char *buff, int=
bufflen)
> +char *owl_keybinding_stack_tostring(int *j, int len)
> =A0{
> - =A0char *pos =3D buff;
> - =A0int =A0 rem =3D bufflen;
> - =A0int =A0 i, n;
> -
> - =A0for (i=3D0; i < len; i++) {
> - =A0 =A0owl_keypress_tostring(j[i], 0, pos, rem-1);
> - =A0 =A0if (i < len - 1) strcat(pos, " ");
> - =A0 =A0n =3D strlen(pos);
> - =A0 =A0pos +=3D n;
> - =A0 =A0rem -=3D n;
> + =A0GString *string;
> + =A0char keypress[64]; =A0/* TODO: don't hard-code a value here. *=
/
> + =A0int =A0i;
> +
> + =A0string =3D g_string_new("");
> + =A0for (i =3D 0; i < len; i++) {
> + =A0 =A0owl_keypress_tostring(j[i], 0, keypress, sizeof(keypress))=
;
> + =A0 =A0g_string_append(string, keypress);
> + =A0 =A0if (i < len - 1) g_string_append_c(string, ' ');
> =A0 }
> - =A0return 0;
> + =A0return g_string_free(string, false);
> =A0}
>
> =A0/* returns 0 on success */
> -int owl_keybinding_tostring(const owl_keybinding *kb, char *buff, =
int bufflen)
> +char *owl_keybinding_tostring(const owl_keybinding *kb)
> =A0{
> - =A0return owl_keybinding_stack_tostring(kb->keys, kb->len, buff, =
bufflen);
> + =A0return owl_keybinding_stack_tostring(kb->keys, kb->len);
> =A0}
>
> =A0const char *owl_keybinding_get_desc(const owl_keybinding *kb)
> diff --git a/keymap.c b/keymap.c
> index 13a07dc..176ad47 100644
> --- a/keymap.c
> +++ b/keymap.c
> @@ -143,15 +143,16 @@ static void _owl_keymap_format_bindings(const=
owl_keymap *km, owl_fmtext *fm)
>
> =A0 nbindings =3D owl_list_get_size(&km->bindings);
> =A0 for (i=3D0; i<nbindings; i++) {
> - =A0 =A0char buff[100];
> + =A0 =A0char *kbstr;
> =A0 =A0 const owl_cmd *cmd;
> =A0 =A0 const char *tmpdesc, *desc =3D "";
>
> =A0 =A0 kb =3D owl_list_get_element(&km->bindings, i);
> - =A0 =A0owl_keybinding_tostring(kb, buff, 100);
> + =A0 =A0kbstr =3D owl_keybinding_tostring(kb);
> =A0 =A0 owl_fmtext_append_normal(fm, OWL_TABSTR);
> - =A0 =A0owl_fmtext_append_normal(fm, buff);
> - =A0 =A0owl_fmtext_append_spaces(fm, 11-strlen(buff));
> + =A0 =A0owl_fmtext_append_normal(fm, kbstr);
> + =A0 =A0owl_fmtext_append_spaces(fm, 11-strlen(kbstr));
> + =A0 =A0g_free(kbstr);
> =A0 =A0 owl_fmtext_append_normal(fm, " - ");
> =A0 =A0 if (kb->desc && *kb->desc) {
> =A0 =A0 =A0 desc =3D kb->desc;
> @@ -319,8 +320,8 @@ int owl_keyhandler_process(owl_keyhandler *kh, =
owl_input j)
>
> =A0void owl_keyhandler_invalidkey(owl_keyhandler *kh)
> =A0{
> - =A0 =A0char kbbuff[500];
> - =A0 =A0owl_keybinding_stack_tostring(kh->kpstack, kh->kpstackpos+=
1, kbbuff, 500);
> + =A0 =A0char *kbbuff =3D owl_keybinding_stack_tostring(kh->kpstack=
, kh->kpstackpos+1);
> =A0 =A0 owl_function_makemsg("'%s' is not a valid key in this conte=
xt.", kbbuff);
> + =A0 =A0g_free(kbbuff);
> =A0 =A0 owl_keyhandler_reset(kh);
> =A0}
> --
> 1.7.3.3.399.g06ddf
>=20
>=20
>=20
>
---1257051904-1833067552-1299821831=:21079--
