C++ Initialize Class Segmentation Fault

Question

Edit -- I updated the class code:

#include "MenuItem.h"
#include <string.h>

MenuItem::MenuItem(const char* txt, const int len)
{
    this->InitText(txt, len);
}

MenuItem::~MenuItem()
{
    delete[] Text;    
}

void MenuItem::InitText(const char* txt, const int len)
{
    Text = new char[len];
    strcpy(Text, txt);
}

void MenuItem::ResetText(const char* txt, const int len)
{
    if (len)
    {
        if (Text)
        {
            delete[] Text;           
        }
        Text = new char[len];
    }
    strcpy(Text, txt);
}

I am trying to implement an array of class objects, and the class has no empty constructor. When I try to initialize the first element I get a segmentation fault.

To declare and set the array,

MenuItem **menuItems;
menuItems = (MenuItem**) malloc(sizeof (MenuItem) * 8);

In my main method I have:

#include "MainApp.h"
#include <iostream>
using namespace Main_App;

int main(int argc, char** argv) {

    MenuItem **menuItems;
    const char title[] = "         Main Title Here                ";
    const char menu1[] = "  1)  Item One                          ";
    const char menu2[] = "  2)  Item Two                          ";
    const char menu3[] = "  3)  Item Three                        ";
    const char menu4[] = "  4)  Item Four                         ";
    const char menu5[] = "  5)  Help                              ";
    const char menu6[] = "  6)  Exit                              ";

    menuItems = (MenuItem**) malloc(sizeof (MenuItem) * 8);


    *menuItems[0] = MenuItem(title, 40); /*<== segmentation fault occurs here */
    *menuItems[1] = MenuItem(menu1, 40);
    *menuItems[2] = MenuItem(menu2, 40);
    *menuItems[3] = MenuItem(menu3, 40);
    *menuItems[4] = MenuItem(menu4, 40);
    *menuItems[5] = MenuItem(menu5, 40);
    *menuItems[6] = MenuItem(menu6, 40);
    *menuItems[7] = MenuItem("", 40);

    for (int i = 0; i < 8; i++) {
        puts(menuItems[i]->Text);
    }

    printf("At main(). Menu App exiting.\n");
    return 0;

}

The class:

#ifndef MENUITEM_H
#define MENUITEM_H

#include "malloc.h"
class MenuItem {
public:
    MenuItem(const char* txt, const int len);
    ~MenuItem();
    void ResetText(const char* txt, const int len = 0);     
    char *Text;
private:
    void InitText (const char* txt, const int len);
};

#endif /* MENUITEM_H */

/**  MenuItem.cpp file */
#include "MenuItem.h"
#include <string.h>

MenuItem::MenuItem(const char* txt, const int len) {
    this->InitText(txt, len);
}

MenuItem::~MenuItem() {
    if (Text) {
        free(Text);
    }
}

void MenuItem::InitText(const char* txt, const int len) {
    Text = (char*) malloc(len);
    strcpy(Text, txt);
}

void MenuItem::ResetText(const char* txt, const int len) {
    if (len) {
        if (Text) {
            free(Text);            
        }
        Text = (char*) malloc(len);
    }
    strcpy(Text, txt);
}

I can modify the class and add an empty constructor, but I would prefer to not do this, to ensure a C-string is always used in the initialization.

This is compiled in Linux 32-bit g++.

-Matt

Don't mix C and C++. In C++ use `std::sting` and `std::vector` instead of trying to do the allocations yourself. — NathanOliver, Oct 02 '17 at 17:58
To clarify krpra's comment, do not tag C when your question is about C++ and vice-versa. The answers you will get will be different depending on the language. — Christian Gibbons, Oct 02 '17 at 17:58
`(MenuItem**) malloc(sizeof (MenuItem) * 8);` `->` `(MenuItem**) malloc(sizeof (MenuItem*) * 8);` — krpra, Oct 02 '17 at 17:58
`malloc`ing objects is a really bad idea. You get storage, but you do not get construction. You might get away with it because you have a really simple class, but you can't count on `Text` being `NULL`, so the destructor... probably gonna die. — user4581301, Oct 02 '17 at 18:06
Also you are violating the heck out of [the Rule of Three](https://stackoverflow.com/questions/4172722/what-is-the-rule-of-three) — user4581301, Oct 02 '17 at 18:07
Neither of these work: menuItems = (MenuItem**) malloc(sizeof (MenuItem*) * 8); menuItems = (MenuItem**) malloc(1024 * 8); — netcat, Oct 02 '17 at 18:11
@Mattz If you're going to use `new[]` to create a string, create a string class if you want to really learn how to do these things without using `std::string`. If you're going to use `new[]` to create a dynamic array, create a dynamic array class if you want to know how `vector` does this. Otherwise you're going to fall into the trap of writing spaghetti-like logic trying to pair `new` and `delete` calls. — PaulMcKenzie, Oct 02 '17 at 18:50

Stephan Lechner · Accepted Answer · 2017-10-02T18:35:44.960

Your title literal " Main Title Here " is of length 41, i.e. 40 characters + string termination character, but you reserve only 40 bytes. So you are one off and should reserve 41 bytes.

Try:

int main() {
    char l[] = "         Main Title Here                ";
    printf("%ld",sizeof(l));
}

Output:

Another issue is that you declare menuItems as being of type pointer to pointer to MenuItem. Then, with *menuItem[0], you dereference a pointer that is not initialized and therefore points to an invalid object. That's the root cause for the seg fault.

You could either declare menuItem as an array of MenuItems, but then MenuItem would require a default constructor. Another option would be to keep the "pointer to pointer"-metaphor and assign pointers to objects created with new:

menuItems = (MenuItem**) malloc(sizeof (MenuItem*) * 8);

menuItems[0] = new MenuItem(title, 40);
menuItems[1] = new MenuItem(menu1, 40);
menuItems[2] = new MenuItem(menu2, 40);
menuItems[3] = new MenuItem(menu3, 40);
menuItems[4] = new MenuItem(menu4, 40);
menuItems[5] = new MenuItem(menu5, 40);
menuItems[6] = new MenuItem(menu6, 40);
menuItems[7] = new MenuItem("", 40);

Anyway, you mix C-style with C++. I'd suggest to dive into the world of C++ and use std::vector, new, delete, ... and get rid of malloc and free. Also think of using std::string; a lot of issues coming from "small" mistakes in memory management will simply not arise...

This is for an embedded device where the processor's frequency is about 400Mhz, and the dynamic RAM is limited to 32 MB. I would prefer to stick with C-strings, but I am open to the idea of vectors and using std::string. — netcat, Oct 02 '17 at 18:55

C++ Initialize Class Segmentation Fault

1 Answers1