HiveBrain v1.2.0
Get Started
← Back to all entries
patterncMinor

Adding support to Busybox vi for reading file from stdin

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
readingfilestdinbusyboxaddingforfromsupport

Problem

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

Solution

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;
}

Code Snippets

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;
}

Context

StackExchange Code Review Q#96698, answer score: 4

Revisions (0)

No revisions yet.