K&R 1-21 write an "entab" program

From K&R exercise 1-21,


Write a program entab that replaces strings of blanks by the minimum
number of tabs and blanks to achieve the same spacing. Use the same
tab stops as for detab. When either a tab or a single blank would
suffice to reach a tab stop, which should be given preference?

My solution:

#include 
#include 

#define MAXLEN  1000    // max line length
#define TABSTOP 8       // tabstop width

int get_line(char line[], int maxlen);
char * entab(char line[], int len);

int main(void)
{
    char line[MAXLEN];  // input line container
    char *entabbed;     // entabbed version of input line
    int len = 0;        // input line length
    int e_len = 0;      // entabbed line length

    while((len = get_line(line, MAXLEN)) > 0) {
        // produce entabbed version
        entabbed = entab(line, len);
        while (entabbed[e_len] != '\0') {
            e_len++;
        }

        // print entabbed version
        printf("%s", entabbed);
        if (entabbed[e_len-1] != '\n') {
            printf("\n");
        }

        free(entabbed);
    }

    return 0;
}

int get_line(char line[], int maxlen)
{
    int i = 0;  // current index
    int c;      // current character

    while (i  0 && line[i-1] != '\n') {
        while ((c = getchar()) != '\n' && c != EOF);
    }

    return i;
}

char * entab(char line[], int len)
{
    int i;          // index into original line
    int e;          // index into entabbed version
    int spaces;     // buffered whitespace counter
    char c;         // current char
    char *entabbed; // entabbed version

    e        = 0;
    spaces   = 0;
    entabbed = (char *) malloc(len * sizeof(char) + 1);

    for (i = 0; i  0) {
                entabbed[e++] = ' ';
                spaces--;
            }

            entabbed[e++] = c;
        }
    }

    return entabbed;
}

I know there are probably more sophisticated ways of handling the input, but I've restricted myself to material covered by the first

I see some things that could help you improve your program.

Learn a recent version of C

I still have my K&R first edition from 1978, however it would be a very poor choice to use for learning C today. The version of C it teaches has been obsolete for decades. I don't know which edition you're using (looks like a more recent one by the style of code) but it would be best to learn C99 or C11. You may already be doing this, but it would be good to check.

Declare variables closer to where they're used

It may be a result of the book you're reading to learn C (per the previous point) but in modern C, we don't declare all of the variables at the top of the function that will be used in the entire function. Instead, declare the variables closer to where they're first used.

Reorder functions to avoid prototypes

You can eliminate the function prototypes by rearranging the function definitions so that each function is defined before its first appearance. In this case, that simply means moving main last, below the other functions.

Use const where practical

The current version of entab doesn't alter the passed line. This can be clearly indicated by making that parameter const:

char * entab(const char line[], int len)

Avoid frequent memory allocations

Memory allocation operations can be expensive in terms of time and are also yet another thing that can go wrong in terms of running out of memory or forgetting to free the block. With that in mind, there are two obvious choices. One would be to pass in an output buffer and length to entab() and the other is to do the entabbing in place. I'd advocate the latter approach since we know that an entabbed line will never be larger than the input string.

Fix bug #1

The first time through the loop, e_len is 0 and gets incremented to point to the terminating '\0', but the value is never reset to zero for the second and subsequent lines. That's a bug. Instead, I'd recommend changing from this

while (entabbed[e_len] != '\0') {
    e_len++;
}

To this:

for (e_len = 0; entabbed[e_len] != '\0'; ++e_len) 
{}

Fix bug #2

Within the entab function, the terminating '\0' is never copied to the resulting copy of the line. The result is that the line may or may not be properly terminated. This is easily fixed by changing the for loop there from this:

for (i = 0; i < len; i++) {

to this:

for (i = 0; i <= len; i++) {

Eliminate return 0 at the end of main

Since C99, the compiler automatically generates the code corresponding to return 0 at the end of main so there is no need to explicitly write it.

StackExchange Code Review Q#114989, answer score: 15