Arrays and Structs - BUG! - C++

[Kourosh23]

Guys, my code has a bug, don't know where it is and how to fix it ? Any help?

 #include <iostream>
using namespace std;

struct x {
    int* array;
    int size;
};


void test1 (int savingArray, int i){
    // Note that you cannot pass in multiple values for arrays all at once.
    arr.array[i] = savigArray;
    cout << arr.array[i];
}

int main(){

    x arr;
    arr.size = 0;
    arr.array[3] = new int [3];
    
    test1 (1, 0);
    test1 (2, 1);
    test1 (3, 2);
    
    
    return 0;
}

[Chervil]

Line 12
arr is not declared
savigArray is misspelled

Line 20

    arr.array[3] = new int [3];

should be

    arr.array    = new int [3];

Arguably, line 19 / 20 should be

    arr.size = 3;
    arr.array    = new int [arr.size];

or better, use a proper class constructor/destructor.

[Kourosh23]

@Chervil, yes you are right.

I have a question, so you said for line 19-20, it should be:

    arr.size = 3;
    arr.array    = new int [arr.size];    // Why there is no square bracket [ ] for arr.array ? 

Fixed Code:

 #include <iostream>
using namespace std;

struct x {
    int* array;
    int size;
};


void test1 (int savingArray, int i){
    // Note that you cannot pass in multiple values for arrays all at once.
    x arr;
    arr.array[i] = savingArray;
    cout << arr.array[i] << " ";
}

int main(){

    x arr;
    arr.size = 3;
    arr.array  = new int [arr.size];
    
    test1 (1, 0);
    test1 (2, 1);
    test1 (3, 2);
    
    return 0;
}

// Output:  1  2  3 

[mbozzi]

There is no square bracket because a square bracket as it appears in an array declaration is part of the identifier's type.

http://eel.is/c++draft/dcl.array#1

You shouldn't write the type of an identifier unless you want to declare an identifier, e.g., compare int i; int i = 0; // doesn't compile! , vs. int i; i = 0; // just fine! .

Perhaps seeing this might make it clearer:

typedef int A[5]; // typedef A as an integer array of 5 elements
A foo; // declare foo as an int array of 5 elements 

[Chervil]

arr.array is a pointer which has not yet been initialised. The new [] operator returns an address which will be stored in the pointer variable. It would not make sense to try to access an element of the dynamic array using the square bracket syntax before the pointer actually pointed to anything. In addition, each element of the array is an int, and it would be a type mismatch if you tried to store an address in an int.

See tutorial:
http://www.cplusplus.com/doc/tutorial/dynamic/
example:

foo = new int [5];

[Chervil]

There are serious problems in this code.

void test1 (int savingArray, int i){
    // Note that you cannot pass in multiple values for arrays all at once.
    x arr;
    arr.array[i] = savingArray;
    cout << arr.array[i] << " ";
}

arr is a local variable of type x.
the variable arr.array is an uninitialised pointer. It is an error to dereference an uninitialised pointer like this:

    arr.array[i] = savingArray;

or like this:

    cout << arr.array[i] << " ";

[48T7M4Gy]

Seems to encapsulate the array satisfactorily as in the previous OP thread on this. Hopefully the destructor does it's job.

#include <iostream>

struct X {
    int* array = nullptr;
    size_t size = 1;
    
    X(size_t aSize = 1) {
        size = aSize;
        array = new int[aSize]{0};
    }
    
    X(int anArray[], size_t aSize) {
        size = aSize;
        array = new int[size]{0};
        for(size_t i = 0; i < size; i++) {
            array[i] = anArray[i];
        }
    }
    
    ~X(){ delete [] array; }
};

std::ostream & operator<<(std::ostream & out, const X & aX) {
    for (size_t i = 0; i < aX.size; i++)
        out << aX.array[i] << '\t';
    
    return out;
}

void double_up(X &aX){
    for(size_t i = 0; i < aX.size; i++)
        aX.array[i] *= 2;
}

int main() {
    int ary[] = {9,8,7};
    X a(10), b, c(7), d(ary, 3);
    
    for(size_t i = 0; i < a.size; i++)
        a.array[i] = 2 * (int)i;
    
    c.array[4] = -901;
    
    std::cout << a << '\n';
    double_up( a );
    std::cout << a << '\n';
    
    std::cout << b << '\n';
    std::cout << c.array[4] << '\n';
    std::cout << c << '\n';
    
    std::cout << d << '\n';
    double_up( d );
    std::cout << d << '\n';
    
    return 0;
}

0	2	4	6	8	10	12	14	16	18	
0	4	8	12	16	20	24	28	32	36	
0	
-901
0	0	0	0	-901	0	0	
9	8	7	
18	16	14	
Program ended with exit code: 0

[Kourosh23]

@mbozzi,
Thanks for your feedback,

You shouldn't write the type of an identifier unless you want to declare an identifier.

Ok, so if I need to declare an array:
int array [ ]= {1, 2, 3};

and if I want to use it again I do something like:
for (int i = 0; i < 3; i++){
cout << array[i];
}

Am I doing it right ?

----------------------------------------------------------------
@chervil,

Thank your for your response,
I understand arr.array is a pointer, and we can initialize it using the struct

x arr;
arr.array = new int [3];
arr.array = 10;    // arr.array is a pointer, so pointers can get a number like this which is of course not the address . 

Question: Does the element 10 store in the first free array location (e.g. index 0) ? Because I believe the pointer gets the value and transfers it to address that is given to the array.


Also, I don't understand why it is an error ? I am simply passing value to the pointer which is accessing it from the struct. Later after I am done with it, I delete it. If you could maybe show me the correct code , in that case I can relate stuff because I am new to C++ :D

the variable arr.array is an uninitialised pointer. It is an error to dereference an uninitialised pointer like this:

arr.array[i] = savingArray;

-------------------------------------------------------------------------
@kemort,

Thank you for your code, it is interesting, and is definitely worth it for future reference.
Just wondering what do you call this form of writing, I mean
std::ostream or std::cout , is it called Mapping ? , why not use cout << and using namespace std?

[48T7M4Gy]

@kourosh23

std::ostream or std::cout , is it called Mapping ?

No, there is a whole story about this which you can google. It is essentially in lieu of
using namespace std; common in learning environments. std:: etc reduces the likelihood of function name (namespace) clashes by explicit reference to the function namespace eg std:: It is good practice to adopt the explicit form. It is a little harder to get used to and read but it pays off.

eg One among millions of comments on this: http://stackoverflow.com/questions/10950083/why-stdcout-instead-of-simply-cout

[mbozzi]

Ok, so if I need to declare an array

...
Yes. But thinking about it I should have chosen my words more carefully. What I said is probably true ( ! ) but it's not very precise. I think I misunderstood what you were confused about.

If you mean to assign arr by putting empty brackets after the name, you're doing it wrong. It won't compile.

If you want to assign an element that's offset from arr's value, then you'd put brackets with some integral expression in them, like you're doing array[i].

Note -- the type system will catch many errors of this kind, which makes it less of an issue.

Re.: the use of using namespace whatever;
It's usually not a good idea.
Briefly, using namespace opens the door to name collisions which in the worst case may silently change the behavior of your program.

In the case of the standard library or widely implemented APIs, these changes occur across compilers, library versions, platforms, or, more usually, as some third party changes their code.

These issues are even worse than preprocessor macro collisions since they're transparent even to
the preprocessed code.

Bugs like this are terrible to diagnose.

[Chervil]

Perhaps there are too many different things going on here, which makes it harder to gain a simple and clear view of what is going on.

1. use of dynamic memory allocation using new / delete.
2. use of user-defined type, such as struct or class.
3. use of functions, and the passing of parameters.
4. possibly also the use of global declarations.

Let's look at items 2 to 4 briefly.

#include <iostream>

using namespace std;

typedef int x;

void test1 (int n)
{
    x arr;   // arr is not initialised, next line is not determined
    cout << "test1 arr = " << arr << '\n';

    arr = n;
    cout << "test1 arr = " << arr << '\n';
}

int main(){

    x arr;   // arr is not initialised, next line is not determined
    cout << "main  arr = " << arr << '\n';
    
    arr = 3;
    cout << "main  arr = " << arr << '\n';
    
    test1(10);
    cout << "main  arr = " << arr << '\n';
    
}

output:

main  arr = 0
main  arr = 3
test1 arr = 0
test1 arr = 10
main  arr = 3

Here instead of a struct I used a user-defined type using the typedef keyword, to keep things simple. The point to be emphasised here is that the variable arr inside function test1() is a completely separate and independent variable from arr inside main(). Changes made inside main do not affect the value inside test1 and vice versa.

Now let's look at an example of dynamic memory allocation.

#include <iostream>

using namespace std;


void test1 (int n )
{
    int * arr = new int[5];

    arr[n] = 5; 

    cout << "test1 " << arr[n] << '\n';

    delete [] arr;
}

int main(){

    int * arr = new int[5];
    arr[3] = 4567;
    
    test1(3);
    
    cout << "main  " << arr[3] << '\n';
             
    delete [] arr;
}

output:

test1 5
main  4567

Note again that arr in main is completely separate from arr in test1. Note also that there is a matched new [] / delete [] Function test1 creates a local variable, allocates some memory, uses that memory. Then the memory is released before the variable goes out of scope and is destroyed.

That's probably enough for one post. To be continued...

[48T7M4Gy]

I don't want to steal anybody's thunder but this is the context of the OP AFAIK.

It makes the discourse a little simpler despite the ambiguity of 'array' and it's scope/location. Whether it's stiil the case or not is debateable but the purpose originally was to encapsulate an array and its size.

http://www.cplusplus.com/forum/beginner/206703/

[Kourosh23]

@kemort and @mbozzi thanks again guys, helpful ideas and logic that I need to dig deeper understanding in for my future.


@chervil,

I know what I was missing,

the variable arr inside function test1() is a completely separate and independent variable from arr inside main().
.

so that means in main and void functions we got to create
int * arr = new int [5];
if we want to keep both arrays values separate of each other. I tried your example by passing the arr to void test1 as a second parameter, and it seems test1 function changes the original value of array in main. So, creating int * arr = new int[5]; prevents it.

Thanks for your great explanation.

[Chervil]

Kourosh23 wrote:

Thank your for your response,
I understand arr.array is a pointer, and we can initialize it using the struct

x arr;
arr.array = new int [3];
arr.array = 10;    // arr.array is a pointer, so pointers can get a number 
                  // like this which is of course not the address .  

Question: Does the element 10 store in the first free array location (e.g. index 0) ? Because I believe the pointer gets the value and transfers it to address that is given to the array.

Well, the first thing which strikes me about that code is that it causes a memory leak.

At line 2, the address of the newly allocated block of memory is stored in the pointer. Then at line 3, the address is overwritten by some completely different value. That makes it impossible to later use delete [] arr.array; because the address of the memory has been lost.

Does the element 10 store in the first free array location (e.g. index 0) ?

No. it replaces the address of the array. If you wanted to store 10 in array index 0, it would be written like this:

    arr.array[0] = 10;

or possibly like this:

    *arr.array = 10;

Kourosh23 wrote:

Also, I don't understand why it is an error ? I am simply passing value to the pointer which is accessing it from the struct. Later after I am done with it, I delete it. If you could maybe show me the correct code , in that case I can relate stuff because I am new to C++ :D

the variable arr.array is an uninitialised pointer. 
It is an error to dereference an uninitialised pointer like this:

    arr.array[i] = savingArray;

Let's look at a simple example.

    int a;         // define an ordinary integer variable
    int * p;       // define a pointer to an int
    p = &a;        // assign address of variable a to pointer p  
    *p = 3;        // store value 3 in the variable pointed to,
                   // that is, store 3 in variable a. 

That code is ok.
But the code in your example was more like this:

    int * p;       // define a pointer to an int
    *p = 3;        // store value 3 in the variable pointed to,
                   // but what does p point to ???? 

In the last example, the pointer contains garbage, it could point to anything at all. If you attempt to store a value there, the program will attempt to modify the contents of some memory location somewhere. Sometimes the program will crash, but this is not guaranteed to happen, the program could silently corrupt some part of computer memory which may have unpredictable effects, not necessarily immediately.

[Chervil]

Possible example code:

#include <iostream>

using namespace std;

struct x {
    int* array;
    int size;
};


void test1 (x & object, int value, int i)
{    
    object.array[i] = value;
}


int main(){

    x arr;
    arr.size  = 3;
    arr.array = new int [arr.size];
    
    test1 (arr, 1, 0);
    test1 (arr, 2, 1);
    test1 (arr, 3, 2);
    
    for (int i= 0; i< arr.size; ++i)
    {
        cout << arr.array[i] << '\n';
    }
    
    delete [] arr.array;
    
    return 0;
}

see Arguments passed by value and by reference in the tutorial

http://www.cplusplus.com/doc/tutorial/functions/

[48T7M4Gy]

#include <iostream>

using namespace std;

struct x {
    int* array = nullptr;
    int size;
};

void test1 (x anX, int index, int value){
    anX.array[index] = value;
    cout << "index: " << index << " value: " << anX.array[index] << '\n';
}

int main(){
    
    x arr;
    arr.size = 3;
    arr.array = new int [3];
    
    test1 (arr, 0, 88);
    test1 (arr, 1, -207);
    test1 (arr, 2, 3521);
    
    cout << "That's it\n";
    
    return 0;
}

index: 0 value: 88
index: 1 value: -207
index: 2 value: 3521
That's it
Program ended with exit code: 0

Edit: Bit of overlap but arr.size = 3 vs 0 is vital for future use/encapsulation of struct x. error arises because a constructor hasn't been implemented for the struct and should be.

[Chervil]

1 2 3 4 5 6 7 8 9

void test1 (x anX, int index, int value){ if ((index < 0) || (index >= anX.size)) { cout << "index out of range: " << index << '\n'; return; } anX.array[index] = value; cout << "index: " << index << " value: " << anX.array[index] << '\n'; }

index: 1 value: 88 index: 2 value: -207 index out of range: 3 That's it

[48T7M4Gy]

That too!

[Kourosh23]

Guys I got the idea, now I know what I was missing, and i guess it is just the matter of practice and comparing it with your codes.

Since you guys have explained so much stuff with details, I make a new post and take your explanations and posts there for reference, so people can use it more easily because right now only us know what is going on in this post :D

@chervill,

At line 2, the address of the newly allocated block of memory is stored in the pointer. Then at line 3, the address is overwritten by some completely different value

Yes that is what I was missing all this time, so after initializing it using
arr.array = new int [5];
I must be specific about the memory location, I mean index 0.
arr.array[0] = 10;

Also, I noticed new int[5] gives 5 memory locations to the pointer int*

Just a further back up for future reference if anyone wants to use:

#include <iostream>
using namespace std;

int main(){
    int size = 5;
    int* array = new int [size];
    array [0] = 2;
    
    for (int i = 0; i < size; i++){
        cout << array[i] << " " << i << endl;
    }
    return 0;   
}
[output]
2 0    // array[0] = 2;
0 1    // The rest are because of new int[5];
0 2
0 3
0 4
[/output]

Also for your next explanation,
so addresses will go to pointers, and pointers can also get an element value and send it to a variable (e.g., *p = 3; to p = &a;). Got it!



@kemort,

I also thought of doing the array parameter (x anX) like the one you did, but I forgot to initialize it using x from the struct. Good point!

void test1 (x anX, int index, int value){
    anX.array[index] = value;
    cout << "index: " << index << " value: " << anX.array[index] << '\n';
}

[48T7M4Gy]

Another aspect to throw into the melting pot is the (gulp!) error checking. eg out of range error trapping. (another gulp)

The point being is STL containers like <vectors> et al do all this work for the end user, and efficiently too if you trust the developers of them. resizing is another sort-of-nightmare left to the STL panoply of wonders.

@Kourosh23,
As far as parameter naming somebody put me onto the idea some time ago for aX or anX, anIndex, aValue even, to differentiate. It's just a convention but reminds the programmer what's going on.
:)