Dynamic allocation of a struct array

Question

I am trying to make a knockoff string struct that will give me the bare bones of what I need for my code (I don't need everything and want to make my code as fast&small as possible). Thus, other than grabbing source for strcpy and strcmp (am I allowed to do that?) I have made a struct hstring to help with my code. So far I have the following for the struct:

struct hstring{
private:
    char *s; // pointer to what holds the string
    int size; // size of the string
public:
    hstring(){
        s=(char *)malloc(0);
        size=0;
    }
    void set(const char* str){ // set the string
        size=0;
        while(str[size]!='\0')
            size++;
        s=(char*)realloc((void *)s,size*sizeof(*s)); // reallocate memory to hold just enough for the character array
        for(int i=0;i<size;i++)
            s[i]=str[i];
        s[size]='\0';
    }
    bool is(const char* str){ // check if something is equal to the string
        int i=0;
        while((s[i]==str[i])&&(str[i]!='\0'))
            i++;
        if((i==size)&&(str[i]=='\0'))
            return true;
        return false;
    }
    inline char* get(){ // return the string
        return s;
    }
    inline int length(){ // return the size of the string
        return size;
    }
};

I've noticed that the only way the set() function works is if I put an explicit string in there or don't have an array. For ex.

// This works
printf("\nTest1\n");
hstring test;
char tmp_c[50];
scanf("%s",tmp_c);
test.set(tmp_c);
printf("%s\n",test.get());

// This works
printf("\nTest2\n");
hstring test2[2];
test2[0].set("Hello ");
test2[1].set("world!");
printf("%s %s\n",test2[0].get(),test2[1].get());

// This works
printf("\nTest3\n");
hstring test3[2];
scanf("%s",tmp_c);
test3[0].set(tmp_c);
scanf("%s",tmp_c);
test3[1].set(tmp_c);
printf("%s %s\n",test3[0].get(),test3[1].get());

// This, what I want to do, does NOT work
printf("\nTest4\n");
hstring *test4 = (hstring *)malloc(2*sizeof(hstring));
for(int i=0;i<2;i++){
    scanf("%s",tmp_c);
    test4[i].set(tmp_c);
}
printf("%s %s",test4[0],test4[1]);
free(test4);

I'm confused as to why the fourth test doesn't properly run. It compiles but crashes upon reaching test4 and trying to reallocate memory in the .set() function. I get an "access violation reading location" error which made me assume I was writing/reading somewhere that I'm not supposed to; however, I cannot determine the exact cause (though I can tell the line causing the error is s=(char*)realloc((void *)s,size*sizeof(*s)); when trying to reallocate the size of the character array. Does someone notice an issue that I have overlooked?

Answer 1

For test4, you use malloc() to allocate memory for two objects:

hstring *test4 = (hstring *)malloc(2*sizeof(hstring));

However, the constructor is not called for either of them. So, neither test4[0] nor test4[1] have been properly initialized. Your class methods probably assume that the referenced object is initialized, and so you get non-deterministic behavior.

The way to fix this is to not use malloc() to allocate your test4, but use std::vector instead:

std::vector<hstring> test4(2);

Then, you can remove your call to free(), as test4 will be destructed properly when the object falls out of scope.

The hstring class itself is managing a pointer to allocated memory. Because of that, you will need to define a destructor for it that will free that memory.

struct hstring {
    //...
    ~hstring () { free(s); }
    //...
};

However, because the destructor has become necessary, it is now also necessary to define a copy constructor, and an assignment operator for your hstring class. This is known as the Rule of Three.

You can avoid this complexity by utilizing an object to manage the memory for you. For your hstring class, it is probably easiest to make the internal char * a std::vector<char> instead. Then there is no need for a destructor, copy constructor, and assignment operator.

Answer 2

Passing 0 to malloc either returns NULL or special pointer which can be used to call free, it is better to make it initialize simply with NULL or better an empty string with a '\0' character.

You are printing printf("%s %s",test4[0],test4[1]); that is printing the structure itself. To print the string you should do printf("%s %s",test4[0].get (),test4[1].get ());

Also, you have a memory leak in the program. You malloced and realloced the s pointer in the hstring structure objects, but never freed them.

I noticed the constructor was not called when you do a malloc. You need to use new/delete instead of malloc/free

For the minimum change you need to do: hstring *test4 = new hstring[2]; when allocating and delete [] test4 when freeing.

See jxh's suggestions also.