Writing a tail command clone

Question

I'm reading Bruce Molay's book on Unix programming, and as an exercise I've implemented a copy of the tail command.

My approach goes over the entire file once, to count newlines, then again to store their positions in an array, and finally a third time to print out the bytes past the position of the nth newline. I know this must be inefficient, though I'm not sure of the ideal way to go about it.

Another point of uncertainty―to get around seeking restrictions on stdin, I write a copy of it to a temp file, then perform all operations on that. I think there's probably a way to do this without a temp file, though I'm not sure what it is.

tail1.c

#include <stdio.h>
#include <unistd.h>
#include <fcntl.h>
#include <stdlib.h>
#include <string.h>
#include <stdbool.h>

#define BUFSIZE 4096
#define N_LINES 10
#define TMPFILE "/tmp/stdin_tmpf.bin"

void oops(char *s1, char *s2) {
    fprintf(stderr, "Error: %s\n(errno) ", s1);
    perror(s2);
    exit(1);
}

unsigned count_chars(const char *str, char byte, int n_chars) {
    int cnt = 0;
    for (int i = 0; i < n_chars; ++i)
        if (str[i] == byte)
            ++cnt;
    
    return cnt;
}

unsigned find_cutoff(int fd) {
    int linecnt, n_chars;
    int subpos, block;
    char cbuf[BUFSIZE];
    subpos = block = linecnt = n_chars = 0;
    
    // count lines to allocate linelocs array
    while ((n_chars = read(fd, cbuf, BUFSIZE)) > 0)
        linecnt += count_chars(cbuf, '\n', n_chars); 
    if (linecnt <= N_LINES)
        return 0;

    // array of positions of newlines
    int linelocs[linecnt];
    linelocs[0] = 0;
    int loc_index = 0;

    if (lseek(fd, 0, SEEK_SET) == -1)
        oops("couldn't seek start", "");

    while ((n_chars = read(fd, cbuf, BUFSIZE)) > 0) {
        for (int i = 0; i < n_chars; ++i)
            if (cbuf[i] == '\n') {
                loc_index++;
                subpos = i+1;
                linelocs[loc_index] = (BUFSIZE*block) + subpos;
            }

        block++;
    }

    return linelocs[linecnt - N_LINES];
}

// create temporary file holding stdin contents
int stdin_tmpf() {
    int out_fd, in_fd;
    int n_chars;
    char buf[BUFSIZE];

    if ((in_fd = fileno(stdin)) == -1)
        oops("couldn't open stdin", "");
    if ((out_fd = open(TMPFILE, O_RDWR | O_CREAT)) == -1)
        oops("failed to create tmpf", "");
    while ((n_chars = read(in_fd, buf, BUFSIZE)) > 0)
        if (write(out_fd, buf, n_chars) != n_chars)
            oops("read/write error", "stdin_tmpf");

    if (lseek(out_fd, 0, SEEK_SET) == -1)
        oops("seek failure", "stdin_tmpf");
    return out_fd;
}

int main(int argc, char **argv) {
    int in_fd, out_fd;
    bool cleanup = false;

    if (argc == 1) {
        in_fd = stdin_tmpf();
        cleanup = true;
    } else if ((in_fd = open(argv[1], O_RDONLY)) == -1)
        oops("Couldn't open file", argv[1]);
    if ((out_fd = fileno(stdout)) == -1)
        oops("Couldn't open stdout", "");

    unsigned cutoff = find_cutoff(in_fd);
    int n_chars;
    char buf[BUFSIZE];

    if (lseek(in_fd, cutoff, SEEK_SET) == -1)
        oops("couldn't seek cutoff", ""); // TODO int to str
    while ((n_chars = read(in_fd, buf, BUFSIZE)) > 0)
        if (write(out_fd, buf, n_chars) != n_chars)
            oops("couldn't write stdout", "");

    if (close(in_fd) == -1 || close(out_fd) == -1)
        oops("couldn't close files", "");
    if (cleanup && unlink(TMPFILE) == -1)
        oops("failed to cleanup", TMPFILE);

    return 0;
}

Quite new to C programming, I've been a Python programmer for many years. Any tips are greatly appreciated!

Answer 1

You only need one pass

Your solution to have three passes over the input has some big problems. Most notably, if the input is very large, you also need to create a large temporary file. But you now also have to deal with issues surrounding temporary files, like what if I run two instances of tail in parallel? What if someone created a symlink from /tmp/stdin_tmpf.bin to some other file?

You can avoid all this by doing only a single pass over the input. The trick is that you know you only need to remember the last N_LINES, so just create a buffer for N_LINES lines. Start filling the buffer, and once it holds N_LINES lines, when you read in the next line, delete the oldest line. Once you finished reading the input, just write out the contents of the buffer.

Note that this is also what the coreutils tail program does.

Use size_t for sizes, counts and indices

I see you use unsigned and int interchangably for keeping track of counts, like n_chars and cnt in count_chars(). However, the right type to use is size_t. Do this whereever appropriate.

Reporting errors

I see you check every return value and report an error both to stderr and by exitting with a non-zero exit code. That's very good! However, I don't see why you both do a fprintf(stderr, ...) and call perror(). I think one is enough. Also, prefer using EXIT_FAILURE as the exit code.

If you are targetting Linux or BSD only, you might consider calling err() instead of your own oops() function.