3

I'm trying to read buffer in C++ one character at the time until '\n', and initialize char array with these characters using do-while loop. I know I could use cin.getline(), but I want to try it on my own. 

int main()
{
    char buffer [1024];
    int index = 0;
    char temp;

    do 
    {
        cin.get( temp );
        buffer [ index ] = temp;
        index ++;
    }
    while ( temp != '\n' );

    cout << buffer << endl;

    return 0;
}

It gives me incorrect result-the proper text fallow by couple of lines of squre brackets mixed with other weird symbols.

Kary
  • 33
  • 1
  • 1
  • 3
  • BTW, `std::getline()` is simpler to use than `std::istream::getline()` since the latter requires you to pick a buffer size ahead of time. – jamesdlin Jan 10 '10 at 20:02

6 Answers6

6

At first, after whole text you have to append '\0' as end of string

it should look like buffer[ index ] = 0; because you should rewrite your \n character which you append too.

Of course, there are other things which you should check but they are not your main problem

  • length of your input because you have limited buffer - max length is 1023 + null byte
  • end of standard input cin.eof()
Gaim
  • 6,734
  • 4
  • 38
  • 58
  • +1 I'd go for this as it is more explicit that ZT has been done (you don't need to look for the whole buffer to have been set to `0`) – epatel Jan 10 '10 at 20:01
3

You're not null-delimiting your buffer.

Try to change the first line to 

char buffer[1024] = "";

This will set all characters in buffer to 0. Or, alternatively, set only the last character to 0, by doing 

buffer[index] = 0;

after the loop.

Also, (as correctly pointed by others) if the text is longer than 1024 characters, you'll have a buffer overrun error - one of the most often exploited causes for security issues in software.

Adam Woś
  • 2,493
  • 20
  • 21
  • Are you sure that `char buffer[1024] = "";` sets all characters to 0? Is not it equal to `char buffer[0] = 0;`? I don't know, I am asking. I am sure, that this sets all characters to 0 `char buffer[1024] = { 0 };` – Gaim Jan 10 '10 at 20:05
  • Yes, I'm sure. "If there are fewer initializers in a brace-enclosed list than there are elements or members of an aggregate, or fewer characters in a string literal used to initialize an array of known size than there are elements in the array, the remainder of the aggregate shall be initialized implicitly the same as objects that have static storage duration." see e.g. http://stackoverflow.com/questions/453432/difference-in-initalizing-and-zeroing-an-array-in-c-c – Adam Woś Jan 10 '10 at 20:10
  • (Response to a deleted comment) No, it doesn't. Please read what I quoted and google some. This sets the **whole array** to zeros. To be precise: *or fewer characters in a string literal* (which is true, because there's 0 < 1024 characters) *used to initialize an array of known size* (=1024) *the remainder of the aggregate shall be initialized implicitly the same as objects that have static storage duration* (which is zero, since static arrays are initialized to all zeros). – Adam Woś Jan 10 '10 at 20:41
3

Two things:

  1. If the length of the line you are reading exceeds 1024 you write past the buffer which is bad.
  2. If the length is within the limit,you are not terminating the string with null char.

You can trying doing it the following way. This way if you find a fine exceeding the buffer size, we truncate it and also add the null char at the end ouside the loop.

#define MAX 1024

int main()
{
 char buffer [MAX];
 int index = 0;
 char temp;

 do 
 {
  // buffer full.
  if(index == MAX-1)
   break;

  cin.get( temp );
  buffer [ index ] = temp;
  index ++;

 }
 while ( temp != '\n' );

 // add null char at the end.
 buffer[index] = '\0';

 cout << buffer << endl;

 return 0;
}
codaddict
  • 445,704
  • 82
  • 492
  • 529
1

Several issues I noted:

(1) What character encoding is the input. You could be reading 8,16, or 32 bit characters. Are you sure you're reading ASCII?

(2) You are searching for '\n' the end of line character could be '\r\n' or '\r' or '\n' depending on your platform. Perhaps the \r character by itself is your square bracket?

Doug T.
  • 64,223
  • 27
  • 138
  • 202
  • 2
    There is the main problem with missing zero byte in the string which ends char sequence. These notes are useful but now they are not important – Gaim Jan 10 '10 at 19:57
0

You stop filling the buffer when you get to a newline, so the rest is uninitialised. You can zero-initialise your buffer by defining it with: char buffer[1024] = {0}; This will fix your problem.

James
  • 24,676
  • 13
  • 84
  • 130
0

You are not putting a '\0' at the end of the string. Additionally, you should really check for buffer overflow conditions. Stop reading when index gets to 1024.

pau.estalella
  • 2,197
  • 1
  • 15
  • 20