[PATCH] vi read-only mode bugfix and enhancement

Natanael Copa natanael.copa at gmail.com
Tue Jul 17 15:42:12 UTC 2007


On Fri, 2007-07-13 at 16:57 +0100, Denys Vlasenko wrote:
> Hi,
> 
> On Thursday 12 July 2007 15:35, Natanael Copa wrote:
> > Attatched is a patch that will fix a bug in vi, enhance functionality a
> > bit, reduce size and make the code a bit more readable. Testcase
> > follows.

[...]

> Nice. Any chance getting "[Read failed]" (or whatever non-puzzling msg)
> so we don't need to return to this once more?

[...]

> Care to do version 2?

More improvements attatched. Fixes also a critical bug (segfault)

* the puzzling message mentioned above is replaced with strerror(errno)
so it should be even more detailed and smaller at the same time.

* merged code in edit_file() and code for ':edit <file>' in colon() into
new func init_text_buffer(). Was horribly duplicate. Moved most of
error/sanity checking to file_insert(). Result is that you get a proper
validation (prevent reading /dev/*) and error messages for ':r <file>' 

* renamed 'cfn' to 'current_filename' for improved readability

* merged smallint vi_readonly and readonly into bitfields into
readonly_mode to save space.

* added text_size variable to keep track how big the text buffer is.
This is used to fix a buffer overflow. To reproduce bug in current svn:

  ./busybox vi TODO
  :r Makefile

vi segfaults due to no buffer checking is done at all. som redesign is
needed here but i added a check in text_hole_make() to aviod the
segfault at least.

* removed isblnk() and use isblank(3) instead.

* fixed compiler warning by displaying the return code for :!<command>
This makes things bigger than needed but since the patch reduces the
overall size... (see below)

* new func next_tabstop(int) merges some duplicate code. There are more
cuplicode here but i couldnt find a good way to merge them.

* Fix *ANNOYING* placement of cursor on '\t' characters. To reproduce in
current svn:
  echo -e "\thello" > file1
  ./busybox vi file1

Try to insert some text at the beginning of line. Text will be inserted
but cursor is blinking somewhere else. The patch should make busybox vi
behave more like original vi(m). Costs a few bytes but its worth it
imho.

* new_text() is moved into init_text_buffer()

* the previously added update_ro_status() was moved info file_insert due
to duplication removal mentioned above.

I'm sure there are some more bugs in there. What happens if you have a
10239 bytes big file and inserts some chars/lines? I think nasty things
might happen. vi also allocates the filesize * 2 which is not very
efficient, but it require big changes so it can be fixed later.

The attatched patch should not make things worse. If somebody find out
that it introduces new bugs or have suggestions to improvements of the
patch, let me know and I'll try to fix.

It needs some testing. Too many changes in one shot.

Now to the bloatcheck:

function                                             old     new   delta
init_text_buffer                                       -     245    +245
file_insert                                          312     420    +108
next_tabstop                                           -      82     +82
text_hole_make                                       154     171     +17
do_cmd                                              5093    5100      +7
static.cmd_mode_indicator                              -       5      +5
refresh                                             1248    1253      +5
current_filename                                       -       4      +4
yank_delete                                          161     164      +3
what_reg                                              96      99      +3
end_cmd_q                                             78      81      +3
char_insert                                          440     442      +2
readonly_mode                                          -       1      +1
vi_readonly                                            1       -      -1
setops                                               154     153      -1
readonly                                               1       -      -1
vi_setops                                              4       1      -3
string_insert                                        161     158      -3
cfn                                                    4       -      -4
show_status_line                                     532     514     -18
readit                                               519     500     -19
move_to_col                                          161     138     -23
vi_main                                              495     433     -62
isblnk                                                75       -     -75
.rodata                                             4751    4655     -96
edit_file                                            892     787    -105
new_text                                             125       -    -125
update_ro_status                                     131       -    -131
colon                                               3848    3667    -181
------------------------------------------------------------------------------
(add/remove: 5/6 grow/shrink: 8/10 up/down: 485/-848)        Total: -363
bytes
   text    data     bss     dec     hex filename
  34751     873    4260   39884    9bcc busybox_old
  34439     877    4260   39576    9a98 busybox_unstripped


-------------- next part --------------
A non-text attachment was scrubbed...
Name: bb-vi-fixes.patch
Type: text/x-patch
Size: 22805 bytes
Desc: not available
Url : http://lists.busybox.net/pipermail/busybox/attachments/20070717/832e3d4c/attachment-0002.bin 


More information about the busybox mailing list