As a C programming exercise, I implemented a minimal tac
program that prints the input line by line starting from the last line.
I'm looking for feedback on my code. Did I overlook anything? Can the style be improved? Did I miss a major issue?
The code is available here or on GitHub.
/*
** tac [ infd [ outfd ]]
**
** output each line from infd to oufile, starting from the last line.
** infd and outfd default to stdin and stdout, respectively.
**
*/
#include <stdio.h>
#include <fcntl.h>
#include <stdlib.h>
#include <unistd.h>
#include <sys/types.h>
#include <errno.h>
void
err_exit(char *msg)
{
perror(msg);
exit(errno);
}
/*
** write each line starting from the end of the buffer.
**
** if flush is falsy, don't output characters unless they are preceded by a newline.
** if flush is truthy, write all characters to the output.
**
** return the number of charaters not outputed.
*/
int
rev_write_lines(int outfd, const char *buf, size_t len, int flush)
{
int i, prev_eol;
size_t line_len;
for(i = prev_eol = len; i >= 0; i--)
if(buf[i] == '\n') {
line_len = prev_eol - i;
if(write(outfd, buf+i+1, line_len) != line_len)
err_exit("write");
prev_eol = i;
}
if(flush) {
if(write(outfd, buf, prev_eol+1) != prev_eol+1)
err_exit("write");
return 0;
}
return prev_eol+1;
}
#define BUF_SIZE 512
/*
** read size characters from stdin into buffer.
** return the number of characters read.
** stop reading when buffer is full or when EOF is encountered.
*/
size_t
read_in_buf(char *buf, size_t size)
{
char *p = buf;
size_t num_read;
int c;
if(size == 0)
return 0;
while((c = getchar()) != EOF) {
*p++ = c;
if(++num_read == size)
break;
}
return num_read;
}
int
stdin_tac(int outfd)
{
size_t buf_size = BUF_SIZE;
char *buf;
ssize_t num_read;
size_t total_read, read_n;
if((buf = (char *) malloc(buf_size)) == NULL)
err_exit("malloc");
total_read = 0;
read_n = buf_size;
while((total_read += num_read = read_in_buf(buf+total_read, read_n))
, num_read == read_n) {
if((buf = (char *) realloc(buf, buf_size *= 2)) == NULL)
err_exit("realloc");
read_n = buf_size - total_read;
}
if(num_read == -1)
err_exit("read(stdin)");
return rev_write_lines(outfd, buf, total_read, 1);
}
int
tac(int infd, int outfd)
{
char *buf;
size_t buf_size = BUF_SIZE, bytes_left;
off_t offset, seek_by;
ssize_t num_read = 0;
int chars_left_in_line = 0;
if((buf = (char *) malloc(buf_size)) == NULL)
err_exit("malloc");
offset = lseek(infd, 0, SEEK_END);
seek_by = -buf_size;
/* loop until we try to seek before the start of file */
while(offset + seek_by >= 0) {
if((offset = lseek(infd, seek_by, SEEK_CUR)) == -1)
err_exit("lseek(SEEK_CUR)");
if((num_read = read(infd, buf, buf_size)) != buf_size)
if(num_read == -1)
err_exit("read(infd)");
chars_left_in_line = rev_write_lines(
outfd, buf, num_read, 0);
if( chars_left_in_line == num_read) {
/*
** buffer is too small to hold this entire line.
** realloc a larger buffer, and (inneficiently) re-read
** the current chunk that we just read in the next iteration.
*/
if((buf = (char *) realloc(buf, buf_size *= 2 )) == NULL)
err_exit("realloc");
seek_by = -buf_size;
} else
seek_by = -2*buf_size + chars_left_in_line;
}
bytes_left = offset + chars_left_in_line;
if((offset = lseek(infd, 0, SEEK_SET)) == -1)
err_exit("lseek(SEEK_START)");
if((num_read = read(infd, buf, bytes_left)) != bytes_left)
err_exit("read");
return rev_write_lines(
outfd, buf, num_read, 1);
}
int
main(int argc, char **argv)
{
int infd, outfd;
if(argc == 1)
return stdin_tac(STDOUT_FILENO);
if((infd = open(argv[1], O_RDONLY)) == -1)
err_exit("open infd");
if(argc == 2)
return tac(infd, STDOUT_FILENO);
if((outfd = open(argv[2], O_CREAT | O_WRONLY, S_IRWXU)) == -1)
err_exit("open outfd");
return tac(infd, outfd);
}
A concession I made is that, on occasion, it is acceptable (though inefficient) to re-read characters that have already been read. For instance, when the buffer overflows in the middle of a line, we re-read the end of the line into a larger buffer as we attempt to read the rest of the line. An alternative would be to copy the incomplete line to the end of the buffer, and read this many less characters. I found that this solution was significantly more complicated to implement, for a negligible performance gain.
gcc
, at a minimum use:-Wall -Wextra -Werror -Wconversion -pedantic -std=gnu11
) Note: Other compilers have their own unique options for enabling the warnings. \$\endgroup\$