Some problem using pointers to enter a string.

Question

Hi all , I'm a beginner and i need to ask a question.. I wrote this small code that accepts a string from the user and prints it..very simple.

#include <iostream>

using namespace std;

int main()
{
   int i;
   char *p = new char[1];

   for(i = 0 ; *(p+i) ; i++) 
     *(p+i) = getchar();

   *(p+i) = 0;

   for(i = 0 ; *(p+i) ; i++) 
     putchar(*(p+i));

   return 0;
}

when i enter any string..like "stack overflow" for example..it will print "sta" and drop the rest of the string. I know it's an easy one to solve but since I've just started i can't understand what's wrong here . Thanks in advance .

Answer 1

There are several problems with this code. First, you have a buffer overflow, because char *p = new char[1] allocates only one character for storage. This is exceeded when i > 0. Next, your first loop will keep going until it reaches a point in unallocated memory (undefined behavior) that has a value of zero. This just happens to be after the third value in your case. You probably wanted something more like *(p+i-1) == 0 to give "the last character read meets some condition." Finally, you're allocating memory with new[] and not properly deallocating it with a matching delete[].

Consider using std::cin and std::string for much safer and correct code:

#include <iostream>
#include <string>

int main(int, char**) {
   std::string s;

   std::cout << "Enter a string: ";
   std::cin >> s;

   std::cout << s << std::endl;
}

Answer 2

The fact that your program does anything is just luck; what stops *(p+i) from being \0 to begin with? It's weird that you're using getchar() and putchar() in a C++ program, too. What's the story behind this program?

Answer 3

you only allocate space for one character but you try to put many chars in it.

Is this homework? if so please tag it as such. Are you allowed to use STL?

If so then use std::vector instead on new char[1];

EDIT:to do it without any fiddly bits or STL

  const int MAX = 100;
  char *p=new char[MAX];
  for(i = 0 ; *(p+i) && i < MAX ; i++) 
      *(p+i) = getchar();

probably some out by ones - left as exercise

Answer 4

If you read into memory, be sure that you allocate enough. new char[1] creates an array of only one char, but you are reading more then that. A simple temporary fix would be to simply allocate more, say new char[255].

Other notes:

• you never delete the memory you allocated: delete[] p;
• you should check wether you read as much characters as your buffer can hold: for(..;.. && i<bufferSize;..)
• the condition in the first loop always checks the next character, not what you just read
• *(p+i) is equivalent to p[i], which is more readable
• why read and write only one character at a time?
• why not use iostreams (std::in, std::out) and std::string as you are using C++?

Answer 5

Here is some code along your lines that seems to work. I'm sure there are better (and more C++-ish) ways to do this...

#include <iostream>
using namespace std;

#define MAXLEN 80

int main()
{
    int i=0;
    char c;

    char *p = new char[MAXLEN + 1];  // 1 char will not be sufficient

    do  // Doing this with a for loop would be unreadable
    {
         c = getchar();
         *(p+i) = c;
         i++;
    } while( c != '\n' && i < MAXLEN ); // Check for a newline. How do you enter the zero with a keyboard?
    *(p+i) = 0; // Ensure that the last character is zero

    for(i = 0 ; *(p+i) ; i++) putchar(*(p+i));  // This is OK but difficult to read

    delete [] p;  // Don't forget this

    return 0;
 }