K&R2 solution to exercise 1-18 (discarding trailing whitespace and empty lines)

Question

Recently I have started learning C++ and I decided to learn C along the way to improve my understanding of how computers work. For C I am using "The C Programming Language Book" by Brian Kernighan and Dennis Ritchie.

The exercise in question is to print all input lines disregarding trailing whitespace and empty lines.

Here is my code:

#include <stdio.h>
#include <stdlib.h>
#define MAX_BLANKS 5

int main()
{
    int c; /*holds current char to be printed*/
    int* wsbuffer; /*buffer to hold possible trailing whitespace*/
    int buffer_size; /*size of wsbuffer*/
    int nws; /*number of whitespaces in buffer*/
    int nonempty; /*0 if current line is empty, 1 else*/
    int i; /*for loop*/
    nonempty  = nws = 0;
    buffer_size = MAX_BLANKS;
    wsbuffer = (int*) malloc(buffer_size*sizeof(int));

    while (EOF != (c=getchar()))
    {
        if (c==' ' || c=='\t')
        {
            ++nws;
            if (nws>buffer_size)
            { 
                buffer_size *=2;
                wsbuffer = (int*) realloc(wsbuffer,buffer_size*sizeof(int));
            }
            wsbuffer[nws-1] = c;
        }
        else if (c=='\n')
        {
            if (nonempty==1) {
                putchar(c);
                nws = nonempty = 0;
            }
        }
        else
        {
            nonempty = 1;
            for (i=0;i<nws;++i)
                putchar(wsbuffer[i]);
            nws = 0;
            putchar(c);
        }
    }
    free(wsbuffer);
    return 0;
}

I compiled this using gcc -std=c90 -Wall -Wextra -pedantic to adhere to the ISO protocol used by the book. It compiled fine with no warnings.

I deliberately set MAX_BLANKS to 5 to test the code. I tested a few cases (empty lines, lines with only space, spaces at first etc) and the result seemed okay.

Since memory allocation is not yet covered in the book, I'd particularly like it if someone could take a look and let me know if I have any issues particularly with memory leaking or something. Of course, any other suggestions or improvements would be welcome!

Answer 1

Don't cast the return value from malloc and family - provided there's a prototype in scope (which there is here, due to our #include <stdlib.h>), the void* result will convert to any pointer type.

Always check whether the returned pointer is a null pointer before using it:

wsbuffer = malloc(buffer_size * sizeof *wsbuffer);
if (!buffer) {
    fputs("Memory allocation failure\n", stderr);
    return EXIT_FAILURE;
}

Robust code needs to take extra care when using realloc(). If we write something like p = realloc(p, new_size); then we have a problem when it fails - p is assigned null, and we have nothing pointing to the memory any more (i.e. a memory leak). The usual pattern looks something like:

void *new_buf = realloc(wsbuffer, buffer_size * sizeof *wsbuffer);
if (!new_buf) {
    free(wsbuffer);
    fputs("Memory allocation failure\n", stderr);
    return EXIT_FAILURE;
}
wsbuffer = new_buf;

---

Minor things:

• Instead of c==' ' || c=='\t', we might consider isspace(c), remembering that this test includes newlines and other whitespace.
• We can store whitespace in an array of char rather than int, since we never need to store EOF into that buffer.
• The logic might be simpler if use a larger buffer and read a whole line at a time. Then we just overwrite the start of the last whitespace found in the line, and print it. That's likely a bit more efficient than our character-at-a-time operation, too.

Answer 2

This is in addition to @TobySpeight review.

Standard Symbolic Constants
Since stdlib.h is included, you have access to EXIT_SUCCESS and EXIT_FAILURE which are standard macros implemented on all systems. These work in C++ as well as C. These are more portable than return 0 or return 1. Most modern C compilers will append return 0; so it really isn't necessary in this particular case. You may have noticed the use of EXIT_FAILURE in @TobySpeight answer.

Sizeof in malloc and realloc
A safer practice is to use what the variable points to rather than a specific type. This will allow you to change the type of the array without changing each malloc or calloc.

wsbuffer = malloc(buffer_size * sizeof(*wsbuffer));

As @TobySpeight mentioned the return value of every memory allocation should be tested for NULL.

Boolean Values
Somewhere in the book you should find:

#define TRUE 1
#define FALSE 0

This might make the code more readable. The C standard has progressed; since the second version of the book was written there is an additional header file that can be used for Booleans, <stdbool.h>.

Programming Style
Generally it is better to wrap operators in expressions in spaces to make the code more readable.

    for (i = 0; i < nws; ++i)

    if (c == ' ' || c == '\t')

For code maintenance reasons, a safer practice when using control constructs (loops, if, else) is to always enclose even a single statement within a block ({..}):

        for (i = 0; i < nws; ++i) {
            putchar(wsbuffer[i]);
        }

That reduces bugs introduced during code maintenance where a second line needs to be added within the control block.

Variable Declarations
When the book was written, all variables had to be declared at the top of the function. This isn't true anymore.

In C, as in C++, variables can be declared as they are needed. For instance a loop control variable such as i can be declared just before the loop.