GNOME Bugzilla – Bug 599863
Segfault when >4kb entered in gjs console
Last modified: 2011-06-04 15:24:22 UTC
GJS expects that 4KiB buffer is large enough which is not true always. Code of a JS library in compressed form (with all white space deleted) can be much larger than 4KiB. Pasting of such large string to GJS console causes segmentation fault. Problem is in this code in gjs/modules/console.c:gjs_console_readline =================================================== line = readline(prompt); ... strcpy(bufp, line); // Segfault when size of line is much larger than size of buffer free(line); =================================================== IMHO, gjs_console_readline should just return string instead of copying it to buffer without checking of buffer bounds.
Created attachment 178892 [details] [review] avoid the segfault Dinamically reallocate memory instead of having a fixed buffer. Making so allows us not to have any line length limit. With this patch, it does not segfault anymore with for example jquery-min-1.4 (with which it did before)
Created attachment 178893 [details] [review] avoid the segfault Typso ...
Review of attachment 178892 [details] [review]: Thanks for looking at this. Typo in commit message, it's "dynamically". ::: modules/console.c @@ +178,3 @@ do { + g_free(buffer); + buffer = NULL; I'd prefer to make buffer a GString, which makes memory handling a lot saner. http://library.gnome.org/devel/glib/2.26/glib-Strings.html#GString @@ +188,3 @@ */ startline = lineno; do { Also, cleaner to allocate here, since the variable is local to the loop. char *temp_buf = NULL; @@ +200,3 @@ + buffer = g_malloc0(buffer_len * sizeof(char)); + strcat(buffer, bufp); + g_free(bufp); This bit is pretty gross, if we were using a GString it'd just be: g_string_append (buffer, bufp); Also, can we call it "temp_buf" instead of "bufp"? @@ +205,3 @@ + if (buffer) + script = JS_CompileScript(context, object, buffer, strlen(buffer), "typein", Then this would be: buffer->str, buffer->len @@ +233,3 @@ } while (!eof); + g_free(buffer); g_string_free(buffer, TRUE);
Created attachment 178911 [details] [review] avoid the segfault Use a GString for the buffer
Review of attachment 178911 [details] [review]: Looks a lot better, no? =) Thanks for the update, will push.
Yep, thx for pointing me at that ! Never used GStrings before
This was pushed