[PATCH] vi read-only mode bugfix and enhancement

[Denys Vlasenko]

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.
>
>   echo "this file is readonly" > file1
>   chmod 400 file1
>   sudo chown root file
>
> Before patch:
>
>   ./busybox_old vi file1
>
> File is empty (user has no read permissions) and statusline says:
>
>   - file1 1/1 100%
>
> Note that there is no indication of error or [Read-only]
>
> After patch:
>
>   ./busybox vi file1
>
> File is still empty (user has no read permissions) but status line says:
>
>   - file1 [Read-only] 1/1 100%
>
> Original vim shows "permission denied" but "Read-only" is better than
> nothing.

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

> Try edit file and save as root. Old behaviour let user do this even if
> file is write protected.
>
> After patch file will be marked as [Read-only]. root will need to save
> file with :wq!

Is this feature important? I ask because it costs a few bytes in code size 
here if I understand it right:

+static void update_ro_status(const char *fn) {
+       struct stat sb;
+       if (stat(fn, &sb) == 0) {
+               readonly = vi_readonly || (access(fn, W_OK) < 0) ||
+                       /* root will always have access()
+                        * so we check fileperms too */
+                       !(sb.st_mode & (S_IWUSR | S_IWGRP | S_IWOTH));
+       }
+}


Small simplification here:

+USE_FEATURE_VI_READONLY(static void update_ro_status(const char *));
 static int file_write(char *, char *, char *);
 static void place_cursor(int, int, int);
 static void screen_erase(void);
@@ -419,8 +420,9 @@
        new_text(size);         // get a text[] buffer
        screenbegin = dot = end = text;
        if (fn != 0) {
-               ch = file_insert(fn, text, cnt);
-       }
+               ch = file_insert(fn, text);
+               USE_FEATURE_VI_READONLY(update_ro_status(fn));
+       }

Use:

#if FEATURE_VI_READONLY
static void update_ro_status(const char *);
#else
static ALWAYS_INLINE void update_ro_status(const char *name) {}
#endif

and call sites don't need USE_FEATURE_VI_READONLY(...) anymore.
Looks better.

Care to do version 2?
--
vda