Print the number of vowels in C

Question

I created this program to print the number of vowels of a given text in C; it is only a small exercise. I consider a vowel only the letters "aeiou", both in upper case and lower case.

I provided a small command line interface: if you give -h or --help you get the help message, if you give the text (e.g. vowel "This is a text") it directly prints the number of vowels. If you run the program without any argument or text, it reads from the stdin until the user types CTRL-D two times.

Examples:

user@computer:$ ./vowels
This is  
a
text
4
user@computer:$ ./vowels "This is a text"
4

The source code is organized in a main.c and a Makefile.

Makefile

.PHONY = clean all

PROGNAME = vowels

CC ?= gcc

CFLAGS = -Wall -Wextra

all : $(PROGNAME)

$(PROGNAME) : main.o
    $(CC) -o $(PROGNAME) $^

clean :
    $(RM) *.o $(PROGNAME)

main.c

#include <stdio.h>
#include <stdio.h>
#include <stdbool.h>
#include <string.h>
#include <ctype.h> /* For tolower(). */

#define BUFFER_SIZE 1024

/* It returns true if the given character is a vowel.
 *
 * Arguments:
 *      - chr : the character to check.
 *
 * Return value:
 *      True if 'chr' is a vowel, otherwise false.
 *
 * Notes:
 *      The vowels considered are English vowels (a, e, i, o, u), both in
 *      upper case and lower case.
 */
bool is_vowel(char chr)
{
        static const char VOWELS[] = "aeiou";

        chr = tolower(chr);

        char *result = strchr(VOWELS, chr);

        return (!result) ? false : true;
}

/* It returns the number of vowels found in a null terminated byte string.
 *
 * Arguments:
 *      - str : the null terminated byte string.
 *
 * Return value:
 *      It returns the number of vowels found in the string.
 *
 * Warning:
 *      The behavior is undefined if str is not a pointer to a null-terminated
 *      byte string.
 */
int vowels_in(char *str)
{
        int vowels = 0;

        while (*str) {
                if (is_vowel(*str))
                        vowels++;

                str++;
        }

        return vowels;
}

/* It prints the help message.
 */
void print_help(void)
{
        static const char message[] =
"usage: vowels [-h|--help] <text>\n\n"
"It shows the number of vowels (aeiou) found in a given text. If you don't\n"
"specify text in the command line, the program reads text from stdin.\n\n"
;

        printf("%s", message);
}

int main(int argc, char *argv[])
{
        char buffer[BUFFER_SIZE];
        int vowels = 0;

        if (argc == 2) {
                if (!strcmp(argv[1], "-h") || !strcmp(argv[1], "--help")) {
                        print_help();
                        return 0;
                } else {
                        vowels = vowels_in(argv[1]);
                        printf("%d\n", vowels);
                }
        } else if (argc == 1) {
                size_t chars;

                /* Repat until the user type CTRL-D or there is an error. */
                do {
                        chars = fread(buffer, sizeof(char), BUFFER_SIZE, stdin);

                        vowels += vowels_in(buffer);
                } while (chars == BUFFER_SIZE);

                printf("\n%d\n", vowels);
        } else {
                fprintf(stderr, "Invalid options; see 'vowels --help'.\n");
                return -1;
        }

        return 0;
}

Answer 1

Your code is buggy and can quickly invoke undefined behavior.

The function tolower must not be called with a char directly, its argument must be cast to unsigned char first. Read the tolower documentation for details.

bool is_vowel(char chr) {
    int lower = tolower((unsigned char)chr);
    return strchr("aeiou", lower) != NULL;
}

In the part that counts the vowels from stdin, you have a serious bug. You read a large buffer and then check the whole buffer for vowels. But you must only check the part that you have actually read, which can be less.

To demonstrate, create a file of size 1025 that is filled with vowels. Your program will count 2048 vowels in that file. Or even more if you are unlucky. Or it crashes. Now that's what undefined behavior means.

To fix this, remove all the buffering code from your program. Instead, read the input one byte at a time:

size_t count_vowels_in_file(FILE *f) {
    size_t vowels = 0;
    int ch;   // It's important to use "int" here, not "char" or "unsigned char".
    while ((ch = fgetc(f)) != EOF) {
        if (is_vowel(ch)) {
            vowels++;
        }
    }
    return vowels;
}

This code is much simpler than reading buffers, and it is almost as fast, since stdin is already buffered.

In short, you didn't test your code well. Code that reads data must be tested with empty input, short input, long input, random input.

To get comparison data, you can use the following shell command:

tr -cd 'AEIOUaeiou' | wc -c

These commands read from stdin. The tr command deletes all characters except the vowels, and the wc command counts the remaining characters, which are all vowels. For every input file, your C program should produce the same result as the tr | wc command combination.

Answer 2

LGTM. In fact L very GTM. Few notes:

• The usage message shall not hardcode the program name, but rely on argv[0].

• Testing for a vowel is not idiomatic. The

      return (!result) ? false : true;
  

  is a long way to say return result;. The entire function could be collapsed into

      return strchr(VOWELS, tolower(chr));
  

• A vowel-counting loop, while correct, is also not idiomatic. I would expect something like

      char chr;
      while ((chr = *str++)) {
          vowels += is_vowel(chr);
      }
  

• #include <stdio.h> twice seems excessive.