[PATCH] Readline's mimic for reverse history search
kyak
bas at bmail.ru
Mon Jul 11 15:46:54 UTC 2011
It used to apply clean at the time of submission (based on the latest git
commit).
Denys, i appreciate your reviews and comments, but i think i will pull
over at this point and say "it works for me and i'm happy with that".
If someone is interested, he can take it from here.
Though i don't see that someone is too much interested in this patch.
Especially seeing the "brace expansion in ash" thread.
Thank you and take care!
On Mon, 11 Jul 2011, Denys Vlasenko wrote:
> On Monday 04 July 2011 09:10, kyak wrote:
>> Implemented readline's mimic for reverse history search (Ctrl+r).
>> Reworked based on Denys Vlasenko remarks.
>
> patch -p1 </tmp/z.diff
> patching file libbb/Config.src
> Hunk #1 FAILED at 94.
> 1 out of 1 hunk FAILED -- saving rejects to file libbb/Config.src.rej
> patching file libbb/lineedit.c
> Hunk #1 FAILED at 1948.
> Hunk #2 FAILED at 2381.
> 2 out of 2 hunks FAILED -- saving rejects to file libbb/lineedit.c.rej
>
>
>
> + /* matched lines from history are here */
> + char *cmdline_buf;
>
> If you need a comment to explain the role of this variable,
> then you need a better name for the variable.
>
>
> + char *prmt_mem_ptr = xzalloc(1);
>
> ???
>
>
> + /* save the real prompt */
> + char *prmt_mem_ptr_save = xzalloc(1);
>
> Not only operation is pointless and leaks memory, but comment is also
> misplaced at best.
>
>
> + cmdedit_y_add_prev = 0;
> + cmdedit_y_add_cmp = 0;
> ...
> + cmdedit_y_add_cmp = ....;
> + cmdedit_y_add_prev = cmdedit_y_add_cmp;
>
> Redundant assignments.
>
>
> + prmt_mem_ptr = xstrdup(prmt);
>
> You never free it: memory leak.
>
>
> + match_buf = xmalloc(MAX_LINELEN * sizeof(int16_t));
>
> You never free it: memory leak.
>
>
> + cmdline_buf = xmalloc(MAX_LINELEN * sizeof(int16_t));
>
> You never free it: memory leak.
> The only time you use allocated memory, namely here
> in case the search was unsuccessful:
> + len_cmd = mbstowcs(wc, cmdline_buf, sizeof(wc));
> + if (len_cmd < 0)
> + len_cmd = strlen(cmdline_buf);
> you use uninitialized data.
>
>
> +#if ENABLE_UNICODE_SUPPORT
> + /* convert to wide char string,
> + * delete char, then convert back */
> + len = mbstowcs(wc, match_buf, sizeof(wc));
> + if (len < 0)
> + len = 0;
> + wc[len-1] = '\0';
> + wcstombs(match_buf, wc, MAX_LINELEN);
> +#else
> + match_buf[strlen(match_buf)-1] = '\0';
> +#endif
>
> What will happen if match_buf is empty string?
>
> And so on...
>
> --
> vda
>
More information about the busybox
mailing list