Segmentation fault when calling destructor

Question

Hey guys I'm fairly new at C++ and this is for my term project. When I make an IPADDRESS object on the STACK, the main.cpp file will run until the deconstructer member is called for the IPADDRESS object. The first section runs just fine, and the second section returns the same output, just with the segmentation error.

// Main file
#include <bits/stdc++.h>
#include <stdio.h>
#include <stdlib.h>
#include <iostream>
#include <string>
#include "IPInfo.h"

using namespace std;

// Function declaration of display()
void displayInfo(IPADDRESS);

int main(){

    const char* address = "cbc.ca";
    const char* command = "nslookup ";

        char buf[256];
        strcpy(buf, command);
        strcat(buf, address);

        string* Info = new(nothrow) string[256]; // Allocate 256 bytes to this string
        *Info = system(buf); // Allocate the response from the command line to Info (which is on the heap)
    cout << Info << endl << *Info << endl << "===============================" << endl << endl;
    delete[] Info;

    IPADDRESS test("cbc.ca");

    displayInfo(test);

// Displays but never exits the program correctly

    return 0;
}
void displayInfo(IPADDRESS Website){

    string* displayBus;
    displayBus = Website.getInfo();
    cout << displayBus;
}

// Header file
#ifndef IPInfo_H
#define IPInfo_H

#include <bits/stdc++.h>
#include <string>
#include <iostream>

using namespace std;
class IPADDRESS{
private:
    const char* command = "nslookup ";
    string url;
    string info;
    string address;
    int searchFor(string locator); // Find a substring in the getInfo string

public:
    string* getInfo(); // Using the system("nslookup") call, get the info (Will be allocated to heap)
    IPADDRESS(string website);
    ~IPADDRESS();
    string getIPAddress(); // Using searchFor() get rid of unneeded characters and dump the IPAddress to a string
    string getName(); // Also output the name

};

IPADDRESS::IPADDRESS(string website){
    url = website;
}

IPADDRESS::~IPADDRESS(){
}
#endif

Please research the very basic difference between automatic and dynamic allocation, and don't give anything except macros names in `ALL_CAPS` ( and also don't use macros ;-) ) — underscore_d, Aug 05 '20 at 13:15
A `delete` expression should only be used on something that was the result of a corresponding `new` expression. There is no `new` expression that initialised `&address`, `&url`, `&info`, or `command`, so none of them should be used in a `delete` expression. Because of that, the behaviour of your destructor is undefined. — Peter, Aug 05 '20 at 13:18
You shouldn't change the original code that this question pertained to. Now the code you've edited doesn't fit the original issue. You should have posted everything first, and then the answers given would have outlined the mistakes you're making in the destructor, plus additional errors you're making in the rest of your code. — PaulMcKenzie, Aug 05 '20 at 13:56
I know now, as I've said before, I'm new to this community, so thanks. — Ubuntu User, Aug 05 '20 at 14:02
[Why should I not #include ?](https://stackoverflow.com/q/31816095/5910058) / [Why is "using namespace std;" considered bad practice?](https://stackoverflow.com/q/1452721/5910058) — Jesper Juhl, Aug 05 '20 at 14:40

Cory Kramer · Answer 1 · 2020-08-05T13:15:10.900

4

This is not correct

IPADDRESS::~IPADDRESS(){

    delete &address;
    delete &url;
    delete &info;
    delete command;
}

you cannot and should not delete any variable that you did not new.

The default compiler-generated destructor is sufficient and correct here (which you can completely remove, it will be implicitly generated in this case).

~IPADDRESS() = default;  // don't even need this, compiler will generate in this case

edited Aug 05 '20 at 13:15

answered Aug 05 '20 at 13:11

Cory Kramer

114,268
16
167
218

1

Why explicitly default the destructor at all? It's not being suppressed here, and the implicitly generated one is correct. – cigien Aug 05 '20 at 13:13
1

@cigien Good point, I updated my answer to point that out more clearly. – Cory Kramer Aug 05 '20 at 13:15
Thanks for the quick response guys! – Ubuntu User Aug 05 '20 at 13:18
I put in your correct destructor, but I'm still getting a segmentation fault. Have any idea what that could be? – Ubuntu User Aug 05 '20 at 13:20
@UbuntuUser Update your original post with a [mcve], not just the class. Regardless, the destructor is now correct. It shouldn't be doing anything except the default. – PaulMcKenzie Aug 05 '20 at 13:22
I updated the post, thanks guys, I'm new to this stack exchange. – Ubuntu User Aug 05 '20 at 13:34

score 1 · Answer 2 · answered Aug 05 '20 at 13:12

1

delete is for freeing what is allocated via new and you must not use it otherwise.

Currently your posted code contains no new, so the destructor should be:

IPADDRESS::~IPADDRESS(){
}

Also note that you should follow The Rule of Three if you use dynamic memory allocation. It is better to avoid bare dynamic memory allocation in C++.

answered Aug 05 '20 at 13:12

MikeCAT

73,922
11
45
70

The destructor should just be omitted as the default one generated then does the right thing. – underscore_d Aug 05 '20 at 13:17
Why define the destructor? Better to default it, and even better is to use the implicit one. – cigien Aug 05 '20 at 13:17

Segmentation fault when calling destructor

2 Answers2