C6001: using unititialized memory "str[i]"

Question

The program works fine, although the fist printed number is always "3452816845". I have tried initializing "str[i]" by adding curly brackets when defining the array, or by giving it NULL value, but then the first printed number is always "zero", and only then it prints what I entered. Please take a look below:

#include <iostream>
using namespace std;

int main() {

    unsigned* str = new unsigned[1000];

    int cnt = 0;
    char ch;
    int a;

    cout << "Please enter text: ";

    do {

        cin.get(ch);

        if (ch <=57 && ch >=48) {

            int a = ch - '0';
            cnt++;
            str[cnt] = a;
        }

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


    cout << "The entered numbers are: ";

    for (int i = 0; i <= cnt; i++) {

        cout << str[i] << " "; // here is where the error appears
    }

    delete[] str;

    return 0;
}

You do `cnt++;` before `str[cnt] = a;`, so it's already going to be at `1` the first time. Also please read [this](https://stackoverflow.com/q/1452721/10411602). — Blaze, Nov 26 '19 at 10:38
Aside from the correct answer that Blaze gave you, you should really start to use proper container classes like `std::vector`, and give your variables proper names. Writing out "count" is not that much of a hassle and improves readability a lot. — Aziuth, Nov 26 '19 at 11:05

DevSolar · Accepted Answer · 2019-11-26T12:19:28.167

Do not using namespace std;. Especially not in headers, but try to not use it in plain .cpp files either. It's more convenient to debug code that unambiguously tells you which namespace an identifier came from right where that identifier is being used.

unsigned* str = new unsigned[1000];

Since the advent of C++11, "naked" memory allocation like that is frowned upon, and is definitely not necessary here.

You could just use a static array (unsigned str[1000];).
You could use smart pointers (auto str = std::make_unique<char[]>(1000);).
Best choice, use C++ containers, like <vector>, <string>, or (if overhead really bothers you) <array>.

if (ch <=57 && ch >=48) {
    int a = ch - '0';

Do not use "magic numbers" in your code. If you want to know if the character entered is a digit, use isdigit, which is more expressive and works even for non-ASCII encodings that might have their digits at a different location in the code table.

int a = ch - '0';

This isn't wrong, as the standard guarantees this to work for digits. Note that similar arithmetic on characters (the infamous ... - 'a') is frowned upon though, and will break as soon as you leave the realm of strict ASCII-7 encoding.

cnt++;
str[cnt] = a;

C/C++ start counting at zero. You just left the first item in the array uninitialized. The beauty of the post-increment is that you can do it right there where you use the index, i.e. str[cnt++] = a;.

for (int i = 0; i <= cnt; i++)
    cout << str[i] << " "; // here is where the error appears
}

Very C, and also wrong. You didn't initialize str[0], so the first round through that loop accesses uninitialized memory. If you had initialized str[0] (by incrementing cnt only after using it as an index), i <= cnt would go one item beyond what you wrote into str[], again accessing uninitialized memory. A loop should run from 0 to < cnt (not <=).

If you took my earlier advice to use <vector> or <string>, there's a much better way to loop through the items stored in it, the range-for.

#include <iostream>
#include <vector>

int main()
{
    char ch;
    std::vector< int > digits;
    std::cout << "Please enter text: ";

    do
    {
        std::cin.get( ch );

        if ( std::isdigit( ch ) )
        {
            digits.push_back( ch - '0' );
        }

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


    std::cout << "The entered numbers are: ";

    for ( auto & i : digits )
    {
        std::cout << i << " ";
    }

    std::cout << std::endl;

    return 0;
}

score 0 · Answer 2 · answered Nov 26 '19 at 10:39

0

You never initialize str[0], but you output it.

answered Nov 26 '19 at 10:39

David Schwartz

179,497
17
214
278

1

While correct, you really should explain to him that the `cnt++;` before the assignment is causing this. – Aziuth Nov 26 '19 at 11:04

machine_1 · Answer 3 · 2019-11-26T11:16:57.060

The problem is here:

...
if (ch <=57 && ch >=48) {

    int a = ch - '0';
    cnt++;
    str[cnt] = a;
}
...

You are incrementing cnt too early, leaving str[0] uninitialized. You should do:

    if (ch <=57 && ch >=48) {

        int a = ch - '0';
        str[cnt++] = a;
    }

Also you do have a problem in your for loop; You should start from 0 till the last initialized element in the string which is at index cnt - 1. It should be like this:

for (int i = 0; i < cnt; i++) {

    cout << str[i] << " ";
}

or

for (int i = 0; i <= cnt - 1; i++) {
    cout << str[i] << " ";
}

C6001: using unititialized memory "str[i]"

3 Answers3