Adding support to Busybox vi for reading file from stdin

Question

I am writing a patch for Busybox's implementation of vi, the classical command line code editor and predecessor of vim. It can open up a files through filenames given as command line arguments. My patch allows initial text to be loaded in to the buffer through a pipe on standard input too.

# normal usage
vi file.txt
# with my patch ('-' tells vi to read from stdin)
ls -l | vi -

The Busybox project is a minimal implementation of many Unix commands, meant to be run on embedded devices and has a very small executable size. The project has a script to measure the executable size delta of two versions. Below is the output for my current implementation.

function                                             old     new   delta
fd_insert                                              -     292    +292
edit_file                                           1299    1370     +71
init_text_buffer                                     333     398     +65
.rodata                                            88972   89004     +32
------------------------------------------------------------------------------
(add/remove: 1/0 grow/shrink: 3/0 up/down: 460/0)             Total: 460 bytes

init_text_buffer does some initialisation and then calls file_insert to read from the given filename. If it fails, the buffer will be left empty (spare a newline). text is a global pointer to the actual char buffer of the document.

I've modified this function to read from standard input if the filename argument is "-". LONE_DASH is a macro also used in other parts of Busybox.

if (fn && LONE_DASH(fn)) {
    rc = fd_insert(STDIN_FILENO, text);
} else {
    rc = file_insert(fn, text, 1);
}

file_insert was originally the only function to fill the text buffer with initial data. I haven't made any modifications to this function, but I used it as a template for fd_insert.

text_hole_make resizes the buffer if it's too small and returns the pointer delta.

full_read reads characters from a file descriptor into a buffer. The third argument is the maximum characters to read. It returns the number of characters read.

I wrote fd_insert from scratch to mimic the behaviour of file_insert, but to be able to read from file descriptors like STDIN_FILENO.

static int fd_insert(int fd, char *p)
{
    int size = BUFSIZ;
    int cnt = 0;
    int total = 0;

    do {
        total += cnt;
        p += text_hole_make(p + total, size);
        cnt = full_read(fd, p + total, size);
    } while (cnt == size);

    if (cnt < 0) {
        status_line_bold_errno("can't read from fd");
        if (total)  // un-do buffer insert
            p = text_hole_delete(p, p + total - 1, NO_UNDO);
        total = cnt;
    } else {
        total += cnt;
        // shrink unused space
        p = text_hole_delete(p + total, p + total + size - cnt - 1, NO_UNDO);
    }

    return total;
}

I've tried to remove the fd argument to reduce size, but this actually increased the size by 2 bytes (+7 to fd_insert and -5 to init_text_buffer).

There is some duplicated file reading logic in file_insert and fd_insert, but I'm not sure how to extract it. My goal is to reduce the executable size and to make my logic as clear as possible for others to read and maintain.

Answer 1

Can be slightly smaller

I was able to shrink your function a little by combining the two calls to text_hole_delete into one call. I'm not sure what compiler options you are using, but when I used gcc -Os on a 32-bit x86 target, it saved 16 bytes. Here is the modified code:

int fd_insert(int fd, char *p)
{
    int size       = BUFSIZ;
    int cnt        = 0;
    int total      = 0;
    int unusedSize = 0;
    int ret        = 0;

    do {
        total += cnt;
        p     += text_hole_make(p + total, size);
        cnt    = full_read(fd, p + total, size);
    } while (cnt == size);

    if (cnt < 0) {
        status_line_bold_errno("can't read from fd");
        ret        = cnt;
        unusedSize = total;
    } else {
        total     += cnt;
        ret        = total;
        p         += total;
        unusedSize = size - cnt;
    }

    // shrink unused space
    if (unusedSize)
        text_hole_delete(p, p + unusedSize - 1, NO_UNDO);
    return ret;
}