[PATCH] vi read-only mode bugfix and enhancement

Denis Vlasenko vda.linux at googlemail.com
Tue Jul 17 23:10:40 UTC 2007


On Tuesday 17 July 2007 16:42, Natanael Copa wrote:
> > 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.

Wow. thanks!

Applied to svn with minor changes. For example, we were comparing argv[0]
with "view", but argv[0] can be "/bin/view"! Right thing is 

	strncmp(appliet_name, "view", 4)

BTW, any idea why strNcmp? Can it really be "viewsomething"? I'm no expert
on vi...
--
vda



More information about the busybox mailing list