-1

Trying to pass pointer for array:

class aaa{
public:
    int a ;
    int b ;
    std::string c ;
};


void abc(aaa* a [])
{
    *a = (aaa*)malloc(sizeof(aaa)* 5);
    a[0]->c ="ddd" ;
    a[1]->c ="ccc" ;  //crash
    a[2]->c ="eee" ;
}

int main() {

    aaa * a;
    abc(&a);

    cout << "!!!Hello World!!!"<< a++->c << endl; 

    cout << "!!!Hello World!!!"<< a++->c << endl; 
    return 0;
}

On second array element assignment I have crash. Where is the problem? Does malloc not creates enough space?

UPD.

I can't change function void abc(aaa* a []) signature because of some reason. It is not mistakable signature even it looks not nice.

I have updated program according recomendations in answers, but I still have crash in getting second array element member:

cout << "!!!Hello World!!!"<< a[1].c << endl;

Why? What I do wrong in code below?

struct aaa{
public:
    int a ;
    int b ;
    std::string c ;
};


int abc(aaa* a [])
{
    int asize =5;
    *a = (aaa*)malloc(sizeof(aaa) * asize);
    for (int i;i<asize;i++)
    {
        a[i] = new aaa();
    }
    a[0]->c ="ddd" ;
    a[1]->c ="ccc" ;
    a[2]->c ="eee" ;
return asize;
}

int main() {
    aaa * a;
    int asize=abc(&a);

    cout << "!!!Hello World!!!"<< a[0].c << endl;
    cout << "!!!Hello World!!!"<< a[1].c << endl; //crash
    cout << "!!!Hello World!!!"<< a[2].c << endl;

    for (int i=0; i<asize;i++)
    {
        cout << "free "<<i<<endl;
        a[i].~aaa();
    }
    free(a);
    cout << "end"<<endl;
    return 0;
}
vico
  • 17,051
  • 45
  • 159
  • 315
  • 2
    You should be using `new` not `malloc` – Ed Heal Jan 22 '18 at 18:15
  • 4
    Stop using `malloc` in a C++ program. You cannot use it for the class you have anyway, since it is not a POD type. Which begs the question -- who or what recommended doing this? – PaulMcKenzie Jan 22 '18 at 18:15
  • 1
    No, `malloc` doesn't call constructors. Your `c` object is uninitialized. – melpomene Jan 22 '18 at 18:15
  • 5
    Well, I hardly know where to begin. – Lightness Races in Orbit Jan 22 '18 at 18:15
  • 1
    Perhaps look up vectors and references – Ed Heal Jan 22 '18 at 18:16
  • 2
    It's like "I will read my C++ book on usage of `std::string`, and then pick up my `C` book to learn dynamic allocation". That's how schizophrenic it looks using C+ idioms and then resorting to `malloc`. – PaulMcKenzie Jan 22 '18 at 18:18
  • 1
    Can I refer you to the book list: https://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list – Richard Critten Jan 22 '18 at 18:18
  • 1
    `a++->c` were my last words before I was exiled into the unknown. I reside there yet still while Bjarne Stroustrup and the rest of the ISO C++ Commitee look down on me, shaking his head. – Arnav Borborah Jan 22 '18 at 18:21
  • 2
    @ArnavBorborah `free(a - 2);` – melpomene Jan 22 '18 at 18:22
  • "Get space for array with malloc" - No! *Don't* use `malloc`. Use `new[]` to get space for an array (if you absolutely *must* do things this way) and then for goodness sake, store that return value into a smart pointer for automatic cleanup. Even better; look up (and use) `std::array` and `std::vector`, and please read [a good book](https://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list) sooner rather than later. – Jesper Juhl Jan 22 '18 at 18:53

2 Answers2

6

The problems are multifold:

  1. mallocing non-POD types so their constructors don't run (catastrophic)
  2. Failure to free the things you malloc (bad)
  3. mallocing in C++ at all (not stylish)
  4. Passing aaa* a[] when you meant aaa** a (valid but misleading)
  5. No #includes, or namespace qualifier on cout and endl (invalid testcase)

Here's what your program should look like:

#include <vector>
#include <string>
#include <iostream>

class aaa
{
public:
    int a;
    int b;
    std::string c;
};


std::vector<aaa> abc()
{
    std::vector<aaa> result;
    result.reserve(3);

    result.push_back({0, 0, "ddd"});
    result.push_back({0, 0, "ccc"});
    result.push_back({0, 0, "eee"});

    return result;
}

int main()
{
    const auto a = abc();

    std::cout << "!!!Hello World!!!"<< a[0].c << std::endl;
    std::cout << "!!!Hello World!!!"<< a[1].c << std::endl;
    std::cout << "!!!Hello World!!!"<< a[2].c << std::endl;
}

(Live demo)

Or, to keep your five up-front element allocations:

std::vector<aaa> abc()
{
    std::vector<aaa> result(5);

    result[0].c = "ddd";
    result[1].c = "ccc";
    result[2].c = "eee";

    return result;
}

I strongly suggest forgetting everything you know about C, when you write C++.

Lightness Races in Orbit
  • 378,754
  • 76
  • 643
  • 1,055
0

You can use malloc, but you still need to call constructors, for this you need new, this defeats idea of using malloc.

const int asize = 5;

void abc(aaa*& a)
{
    a = (aaa*)malloc(sizeof(aaa) * asize); // you need to release memory later
    for (int i = 0; i < asize; ++i) {
        new (a+i) aaa(); // you need to call constructors to intialize string
    }
    // now you can use strings
    a[0].c = "ddd";
    a[1].c = "ccc";
    a[2].c = "eee";
}

int main() {
    aaa * a;
    abc(a);

    cout << "!!!Hello World!!!" << a[0].c << endl;
    cout << "!!!Hello World!!!" << a[1].c << endl;

    // finally you need to call destructors
    for (int i = 0; i < asize; ++i) {
        a[i].~aaa();
    }
    free(a);
    return 0;
}

After showing you how you can make it work, i would like to propose another solution. If you care about memory and don't want to use std::vector, you can use std::unique_ptr.

std::unique_ptr<aaa[]> data;
data = std::make_unique<aaa[]>(asize);
data[0].c = "text";
cout << data[0].c;
// no need to manually release memory

EDIT: As of updated question. If you really want to pass array of pointers then you can do the following:

const int asize = 5;

void abc(aaa* a[]) {
    // If array is really big, then you probably should preallocate memory and call placement new for every element.
    for (int i = 0; i < asize; ++i) {
        a[i] = new aaa; // again, you have to release memory 
    }
    // now you can use strings
    a[0]->c = "ddd";
    a[1]->c = "ccc";
    a[2]->c = "eee";
}

int main() {
    aaa * a[asize];
    abc(a);

    cout << "!!!Hello World!!!" << a[0]->c << endl;
    cout << "!!!Hello World!!!" << a[1]->c << endl;

    for (int i = 0; i < asize; ++i) {
        delete a[i];
    }
    return 0;
}

It would be very nice if you could use unique_ptr instead of raw pointer.

Yola
  • 18,496
  • 11
  • 65
  • 106
  • Except you leaked it all. – Lightness Races in Orbit Jan 22 '18 at 18:25
  • @LightnessRacesinOrbit well, right, i mentioned it in the comment:) – Yola Jan 22 '18 at 18:26
  • 1
    Mm. Well, while I can't commend the approach or advise recommending this to the OP, you've certainly come up with an otherwise valid solution :P BTW I know it looks like you're "calling constructors" but you're not really; there is no syntax to explicitly "call a constructor". Nitpicking though as the end result is the same, particularly in the placement new case. – Lightness Races in Orbit Jan 22 '18 at 18:27
  • If we're going full C++, shouldn't it be `static_cast(malloc(...))`? – melpomene Jan 22 '18 at 18:27
  • 1
    Actually I do wonder whether alignment constraints are satisfied here – Lightness Races in Orbit Jan 22 '18 at 18:28
  • @LightnessRacesinOrbit hmm, it depends on how std::string implemented, good point! – Yola Jan 22 '18 at 18:31
  • 2
    Alignment? Probably chaotic neutral. – user4581301 Jan 22 '18 at 18:50
  • @user4581301 #igotthatreference – Lightness Races in Orbit Jan 22 '18 at 18:52
  • I can't escape from function `void abc(aaa* a [])` signature. Since that reason I have updated question body with updated code that crashes in the same place. – vico Jan 23 '18 at 15:41
  • @vico hope after my two answers it is more clear for you:) – Yola Jan 23 '18 at 16:14
  • In my live case `main()` not knows what is size of array. So, I can't use line `aaa * a[asize];`. `abs()` should define size and fill array. – vico Jan 24 '18 at 09:43
  • @vico you can't do anything in this case, because parameter of `abc` is an array of pointers and you can do what you want with pointers but not with array. I suggest you to close this question and start new, only describe the real problem, why you do you need such function signature at all, probably the problem is at another layer. – Yola Jan 24 '18 at 10:02
  • I would like to understand why I can't do with pointer to array of pointers and can with pointer. Pointer to array is still pointer and contains not for example `aaa` type, but `[]` of `aaa` that might be retrieved if you know `[]` size. But yes, this is new question. – vico Jan 24 '18 at 13:10
  • @vico you don't have `pointer to array of pointers` your function accepts just `array of pointers`, so it can work with this array, but not change it. Look into my first code sample, i don't pass just a pointer, but reference to a pointer to be able to change this pointer, without reference, i would be able to change only pointee, not pointer. – Yola Jan 24 '18 at 13:20
  • @Yola: _"your function accepts just array of pointers"_ Actually it doesn't. – Lightness Races in Orbit Jan 25 '18 at 21:45
  • 1
    @LightnessRacesinOrbit about alignment: [the C++ standard](http://eel.is/c++draft/c.malloc#2) redirects to the C one, [which says](https://busybox.net/~landley/c99-draft.html#7.20.3) that *"The pointer returned [...] is suitably aligned so that it may be [...] used to access such an object or an array of such objects in the space allocated [...]."* So this is fine as far as alignment is concerned :) – Quentin Jan 26 '18 at 10:05
  • @LightnessRacesinOrbit `aaa *arr[]` - array of pointers, isn't it? – Yola Jan 27 '18 at 09:17
  • @Yola: I'm afraid not. It is automatically translated to `aaa** arr` and actually has nothing to do with arrays. We inherited this ghastly nonsense from C, sorry. – Lightness Races in Orbit Jan 27 '18 at 21:50