6
#include<iostream>
#include<string.h>
#include<stdio.h>


int main()
{
    char left[4];
    for(int i=0; i<4; i++)
    {
        left[i]='0';
    }
    char str[10];
    gets(str);
    strcat(left,str);
    puts(left);
    return 0;
}

for any input it should concatenate 0000 with that string, but on one pc it's showing a diamond sign between "0000" and the input string...!

Zac Howland
  • 15,777
  • 1
  • 26
  • 42
CapeAndCowl
  • 370
  • 2
  • 14
  • 5
    You're strcating beyond the end of your buffer. That's indeed undefined behavior. – Frédéric Hamidi Dec 16 '13 at 14:20
  • 3
    Function [gets()](http://stackoverflow.com/questions/1694036/why-is-the-gets-function-dangerous-why-should-it-not-be-used) is deprecated and dangerous, don't use it. – this Dec 16 '13 at 14:20
  • 2
    left is not a zero-terminated string. You need to define char left[5]; and then add after the loop left[4]=0; // note 0 and not '0' – Giacomo Degli Esposti Dec 16 '13 at 14:21
  • The answer to this question should help you. http://stackoverflow.com/a/8313831/2451162 – Paul Brindley Dec 16 '13 at 14:23
  • 2
    This only appears to work on one pc because the memory layout happens to be `LLLLSSSSSSSSS`, where `L` is space taken by `left`, and `S` is space taken by `str`. There is no guarantee the compiler will lay out your memory like this. My guess is that the second compiler is optimizing your int `i` down to a `uint8_t`, resulting in the memory layout `LLLLISSSSSSSSSS`, hence the mystery diamond `(char) 4` – Eric Dec 16 '13 at 14:29

5 Answers5

6

You append a possible nine (or more, gets have no bounds checking) character string to a three character string (which contains four character and no string terminator). No string termination at all. So when you print using puts it will continue to print until it finds a string termination character, which may be anywhere in memory. This is, in short, a school-book example of buffer overflow, and buffer overflows usually leads to undefined behavior which is what you're seeing.

In C and C++ all C-style strings must be terminated. They are terminated by a special character: '\0' (or plain ASCII zero). You also need to provide enough space for destination string in your strcat call.


Proper, working program:

#include <stdio.h>
#include <string.h>
#include <errno.h>

int main(void)
{
    /* Size is 4 + 10 + 1, the last +1 for the string terminator */
    char left[15] = "0000";
    /* The initialization above sets the four first characters to '0'
     * and properly terminates it by adding the (invisible) '\0' terminator
     * which is included in the literal string.
     */

    /* Space for ten characters, plus terminator */
    char str[11];

    /* Read string from user, with bounds-checking.
     * Also check that something was truly read, as `fgets` returns
     * `NULL` on error or other failure to read.
     */
    if (fgets(str, sizeof(str), stdin) == NULL)
    {
        /* There might be an error */
        if (ferror(stdin))
            printf("Error reading input: %s\n", strerror(errno));
        return 1;
    }

    /* Unfortunately `fgets` may leave the newline in the input string
     * so we have to remove it.
     * This is done by changing the newline to the string terminator.
     *
     * First check that the newline really is there though. This is done
     * by first making sure there is something in the string (using `strlen`)
     * and then to check if the last character is a newline. The use of `-1`
     * is because strings like arrays starts their indexing at zero.
     */
    if (strlen(str) > 0 && str[strlen(str) - 1] == '\n')
        str[strlen(str) - 1] = '\0';

    /* Here we know that `left` is currently four characters, and that `str` 
     * is at most ten characters (not including zero terminaton). Since the
     * total length allocated for `left` is 15, we know that there is enough
     * space in `left` to have `str` added to it.
     */
    strcat(left, str);

    /* Print the string */
    printf("%s\n", left);

    return 0;
}
Some programmer dude
  • 400,186
  • 35
  • 402
  • 621
2

There are two problems in the code.

First, left is not nul-terminated, so strcat will end up looking beyond the end of the array for the appropriate place to append characters. Put a '\0' at the end of the array.

Second, left is not large enough to hold the result of the call to strcat. There has to be enough room for the resulting string, including the nul terminator. So the size of left should at least 4 + 9, to allow for the three characters (plus nul terminator) that left starts out with, and 9 characters coming from str (assuming that gets hasn't caused an overflow).

Each of these errors results in undefined behavior, which accounts for the different results on different platforms.

Pete Becker
  • 74,985
  • 8
  • 76
  • 165
1

I do not know why you are bothering to include <iostream> as you aren't using any C++ features in your code. Your entire program would be much shorter if you had:

#include <iostream>
#include <string>

int main()
{
    std::string line;
    std::cin >> line;
    std::cout << "You entered:  " << line;
    return 0;
}

Since std::string is going to be null-terminated, there is no reason to force it to be 4-null-terminated.

Zac Howland
  • 15,777
  • 1
  • 26
  • 42
0

Problem #1 - not a legal string:

char left[4];
for(int i=0; i<4; i++)
{
    left[i]='0';
}

String must end with a zero char, '\0' not '0'. This causes what you describe.

Problem #2 - fgets. You use it on a small buffer. Very dangerous.

Problem #3 - strcat. Yet again trying to fill a super small buffer which should have already been full with an extra string.

This code looks an invitation to a buffer overflow attack.

egur
  • 7,830
  • 2
  • 27
  • 47
0

In C what we call a string is a null terminated character array.All the functions in the string.h library are based on this null at the end of the character array.Your character array is not null terminated and thus is not a string , So you can not use the string library function strcat here.

Rafed Nole
  • 112
  • 1
  • 4
  • 16