Re: libedit wchar allocation issue
Sure, looks good, I'll use your version.
On Wed, Nov 16, 2011 at 02:00:51PM +0100, Stefan Sperling wrote:
On Tue, Nov 15, 2011 at 11:09:08PM +, Nicholas Marriott wrote:
Hi
libedit miscalculates the amount of space needed for constructing it's
wchar_t version of argv, causing it to overrun the buffer.
I don't see how the output of mbstowcs can be longer than
(sum(strlen(argv)) * sizeof (wchar_t)) so this fix should work:
ok?
Index: chartype.c
===
RCS file: /cvs/src/lib/libedit/chartype.c,v
retrieving revision 1.3
diff -u -p -r1.3 chartype.c
--- chartype.c 7 Jul 2011 05:40:42 - 1.3
+++ chartype.c 15 Nov 2011 00:38:44 -
@@ -147,7 +147,7 @@ ct_decode_argv(int argc, const char *arg
* the argv strings. */
for (i = 0, bufspace = 0; i argc; ++i)
bufspace += argv[i] ? strlen(argv[i]) + 1 : 0;
- ct_conv_buff_resize(conv, 0, bufspace);
+ ct_conv_buff_resize(conv, 0, bufspace * sizeof(*p));
if (!conv-wsize)
return NULL;
OK, your fix is good (with or without my suggestion below).
Even though we have enough space the code should properly evaluate
what mbstowcs() returns, if only to provide a good example.
Renaming the variable isn't strictly needed but 'bytes' is very misleading.
Index: chartype.c
===
RCS file: /cvs/src/lib/libedit/chartype.c,v
retrieving revision 1.3
diff -u -p -r1.3 chartype.c
--- chartype.c7 Jul 2011 05:40:42 - 1.3
+++ chartype.c16 Nov 2011 12:40:13 -
@@ -141,13 +141,13 @@ ct_decode_argv(int argc, const char *arg
int i;
Char *p;
Char **wargv;
- ssize_t bytes;
+ size_t wlen;
/* Make sure we have enough space in the conversion buffer to store all
* the argv strings. */
for (i = 0, bufspace = 0; i argc; ++i)
bufspace += argv[i] ? strlen(argv[i]) + 1 : 0;
- ct_conv_buff_resize(conv, 0, bufspace);
+ ct_conv_buff_resize(conv, 0, bufspace * sizeof(*p));
if (!conv-wsize)
return NULL;
@@ -159,15 +159,16 @@ ct_decode_argv(int argc, const char *arg
continue;
} else {
wargv[i] = p;
- bytes = mbstowcs(p, argv[i], bufspace);
+ wlen = mbstowcs(p, argv[i], bufspace);
}
- if (bytes == -1) {
+ if (wlen == (size_t)-1 || wlen == bufspace) {
+ /* Encoding error or not enough room for NUL. */
el_free(wargv);
return NULL;
} else
- bytes++; /* include '\0' in the count */
- bufspace -= bytes;
- p += bytes;
+ wlen++; /* include NUL in the count */
+ bufspace -= wlen;
+ p += wlen;
}
return wargv;
Re: libedit wchar allocation issue
On Tue, Nov 15, 2011 at 11:09:08PM +, Nicholas Marriott wrote:
Hi
libedit miscalculates the amount of space needed for constructing it's
wchar_t version of argv, causing it to overrun the buffer.
I don't see how the output of mbstowcs can be longer than
(sum(strlen(argv)) * sizeof (wchar_t)) so this fix should work:
ok?
Index: chartype.c
===
RCS file: /cvs/src/lib/libedit/chartype.c,v
retrieving revision 1.3
diff -u -p -r1.3 chartype.c
--- chartype.c7 Jul 2011 05:40:42 - 1.3
+++ chartype.c15 Nov 2011 00:38:44 -
@@ -147,7 +147,7 @@ ct_decode_argv(int argc, const char *arg
* the argv strings. */
for (i = 0, bufspace = 0; i argc; ++i)
bufspace += argv[i] ? strlen(argv[i]) + 1 : 0;
- ct_conv_buff_resize(conv, 0, bufspace);
+ ct_conv_buff_resize(conv, 0, bufspace * sizeof(*p));
if (!conv-wsize)
return NULL;
OK, your fix is good (with or without my suggestion below).
Even though we have enough space the code should properly evaluate
what mbstowcs() returns, if only to provide a good example.
Renaming the variable isn't strictly needed but 'bytes' is very misleading.
Index: chartype.c
===
RCS file: /cvs/src/lib/libedit/chartype.c,v
retrieving revision 1.3
diff -u -p -r1.3 chartype.c
--- chartype.c 7 Jul 2011 05:40:42 - 1.3
+++ chartype.c 16 Nov 2011 12:40:13 -
@@ -141,13 +141,13 @@ ct_decode_argv(int argc, const char *arg
int i;
Char *p;
Char **wargv;
- ssize_t bytes;
+ size_t wlen;
/* Make sure we have enough space in the conversion buffer to store all
* the argv strings. */
for (i = 0, bufspace = 0; i argc; ++i)
bufspace += argv[i] ? strlen(argv[i]) + 1 : 0;
- ct_conv_buff_resize(conv, 0, bufspace);
+ ct_conv_buff_resize(conv, 0, bufspace * sizeof(*p));
if (!conv-wsize)
return NULL;
@@ -159,15 +159,16 @@ ct_decode_argv(int argc, const char *arg
continue;
} else {
wargv[i] = p;
- bytes = mbstowcs(p, argv[i], bufspace);
+ wlen = mbstowcs(p, argv[i], bufspace);
}
- if (bytes == -1) {
+ if (wlen == (size_t)-1 || wlen == bufspace) {
+ /* Encoding error or not enough room for NUL. */
el_free(wargv);
return NULL;
} else
- bytes++; /* include '\0' in the count */
- bufspace -= bytes;
- p += bytes;
+ wlen++; /* include NUL in the count */
+ bufspace -= wlen;
+ p += wlen;
}
return wargv;
Re: libedit wchar allocation issue
On Tue, Nov 15, 2011 at 11:09:08PM +, Nicholas Marriott wrote: Hi libedit miscalculates the amount of space needed for constructing it's wchar_t version of argv, causing it to overrun the buffer. I don't see how the output of mbstowcs can be longer than (sum(strlen(argv)) * sizeof (wchar_t)) so this fix should work: ok? This seems to fix an intermittent problem I have been having with libedit crashing during various initialisation dances. I approve. -- The wages of sin are high but you get your money's worth.
libedit wchar allocation issue
Hi libedit miscalculates the amount of space needed for constructing it's wchar_t version of argv, causing it to overrun the buffer. I don't see how the output of mbstowcs can be longer than (sum(strlen(argv)) * sizeof (wchar_t)) so this fix should work: ok? Index: chartype.c === RCS file: /cvs/src/lib/libedit/chartype.c,v retrieving revision 1.3 diff -u -p -r1.3 chartype.c --- chartype.c 7 Jul 2011 05:40:42 - 1.3 +++ chartype.c 15 Nov 2011 00:38:44 - @@ -147,7 +147,7 @@ ct_decode_argv(int argc, const char *arg * the argv strings. */ for (i = 0, bufspace = 0; i argc; ++i) bufspace += argv[i] ? strlen(argv[i]) + 1 : 0; - ct_conv_buff_resize(conv, 0, bufspace); + ct_conv_buff_resize(conv, 0, bufspace * sizeof(*p)); if (!conv-wsize) return NULL;
