R Sahu
R Sahu

Reputation: 206627

Buggy code in "A Tour of C++" or non-compliant compiler?

I saw the following function in "A Tour of C++", page 12:

int count_x(char const* p, char x)
{
   int count = 0;
   while (p)
   {
      if (*p == x) ++count;
      ++p;
   }
   return count;
}

The line while (p) did not sound right to me. I thought it should've been while (*p). However, not wanting to be too presumptuous, I tested the function with the following code.

int main(int argc, char** argv)
{
   char* p = argv[1];
   char x = argv[2][0];
   int count = count_x(p, x);

   std::cout
      << "Number of occurences of "
      << x << " in " << p << ": " << count << std::endl;
   return 0;
}

When I ran the program, it exited with Segmentation fault (core dumped). I was glad to see that error since the code did not look right to me to start with. However, now am curious. Is the code suggested in the book incorrect or is the compiler not C++11 compliant? The compiler is g++ (GCC) 4.7.3.

What makes the code of count_x strange is that the author, Bjarne Stroustrup, starts with the following implementation before finishing with the one I wrote first.

int count_x(char const* p, char x)
{
   if(p==nullptr) return 0;
   int count = 0;
   for (; p!=nullptr; ++p)
   {
      if (*p == x)
         ++count;
   }
   return count;
}

It made me think twice before concluding this is buggy code. Both versions appear to be buggy.

Upvotes: 16

Views: 1487

Answers (3)

tup
tup

Reputation: 139

This is a corrected version. It checks whether the *p pointer is actually null. It also ensures that there are two input variables.

#include <iostream>

int count_x(char const* p, char x)
{
   int count = 0;
   while (*p) // check if *p is not a null character
   {
      if (*p == x) ++count;
      ++p;
   }
   return count;
}
int main(int argc, char** argv)
{
   if (argc < 3) // check if there are at least two arguments
   {
      std::cout << "Error: Not enough arguments" << std::endl;
      return 1;
   }
   char* p = argv[1];
   char x = argv[2][0];
   int count = count_x(p, x);

   std::cout
      << "Number of occurences of "
      << x << " in " << p << ": " << count << std::endl;
   return 0;
}

Upvotes: 0

Chris Drew
Chris Drew

Reputation: 15334

This is listed in the Errata for 2nd printing of A Tour of C++:

Chapter 1:

pp 11-12: The code for count_if() is wrong (doesn't do what it claims to), but the points made about the language are correct.

The code in the 2nd Printing now reads:

int count_x(char* p, char x)
    // count the number of occurences of x in p[]
    // p is assumed to point to a zero-terminated array of char (or to nothing)
{
    if (p==nullptr) return 0;
    int count = 0;
    while(*p) {
        if(*p==x)
            ++count;
        ++p;
    }
    return count;
}

There is a similar example in The C++ Programming Language (4th Edition) on which A Tour of C++ was based but it does not have this bug.

Upvotes: 6

user26347
user26347

Reputation: 604

gcc has good compatibility at 4.7.3, but you have to compile with -std=c++11. There are charts on the gnu webpage. But yeah, that's not a standard thing, that pointer is just never going to be NULL, at least not until it overflows, so unless you've allocated all the memory above the char *, it's going to segfault. It's just a bug.

Upvotes: 3

Related Questions