C++ Segmentation fault

[benclifford]

I have done this code 5 times in my program for different classes and variables and it has worked fine for 4 of them but the 5th is giving a segmentation fault.

The error occurs just after the cout saying 'here' in the main.cpp code below

I have run valgrind and the result says conditional jump or move depends on uninitalised values

but i cannot see what it is referring.

Could someone please point out where my mistake is and how to fix it.

smurfVillage.h

#ifndef smurfVillage_H
#define smurfVillage_H
#include <string>
using namespace std;

class GenericBuilding
{
  private:
    string building_name;
    string location;
    string functionality;

  public:
    GenericBuilding();
    GenericBuilding(string,string,string);
    ~GenericBuilding();
    string getBuildingName();
    string getLocation();
    string getFunctionality();
    string setBuildingName(string);
    string setLocation(string);
    string setFunctionality(string);
};
#endif

smurfVillage.cpp

    //***********************GenericBuilding Constructors***********************//
    GenericBuilding::GenericBuilding()
    {
      //default parameterless constructor
      building_name = "Building"; 
      location = "City";
      functionality = "Building";
      std::cout << " >> Constructing default GenericBuilding" << std::endl;
    }

    GenericBuilding::GenericBuilding(string Name, string Location, string Functionality)
    {
      //Constructor for specified values
      building_name = Name; 
      location = Location;
      functionality = Functionality;
      std::cout << " >> Constructing GenericBuilding" << std::endl;
    }
    //*******************End of GenericBuilding Constructors********************//

    //************************GenericBuilding Destructor************************//
    GenericBuilding::~GenericBuilding()
    {
      std::cout << " >> Destructing GenericBuilding" << std::endl;
      std::cout << std::endl;
    }
    //********************End of GenericBuilding Destructor*********************//

    //**********************GenericBuilding get functions***********************//
    string GenericBuilding::getBuildingName()
    {
      return building_name;
    }

    string GenericBuilding::getLocation()
    {
      return location;
    }

    string GenericBuilding::getFunctionality()
    {
      return functionality;
    }
    //******************End of GenericBuilding get functions********************//

    //**********************GenericBuilding set functions***********************//
    string GenericBuilding::setBuildingName(string Name)
    {
      building_name = Name;
    }

    string GenericBuilding::setLocation(string Location)
    {
      location = Location;
    }

    string GenericBuilding::setFunctionality(string Functionality)
    {
      functionality = Functionality;
    }
    //*******************End of GenericBuilding set functions*******************//

main.cpp

#include "smurfVillage.h"
#include <iostream>

using namespace std;

int main ()
{
  GenericBuilding *GB1 = new GenericBuilding();
  GenericBuilding *GB2 = new GenericBuilding("Hospital", "City", "Caring");

  std:cout << "GB2 Name: " << GB2->getBuildingName() << std::endl;
  std::cout << "GB2 Location: " << GB2->getLocation() << std::endl;
  std::cout << "GB2 Functionality: " << GB2->getFunctionality() << std::endl;

  std::cout << "here" << endl;

  GB2->setBuildingName("Bank");
  GB2->setLocation("Town");
  GB2->setFunctionality("Withdraweral");

  std::cout << GB2->getBuildingName() << std::endl;
  std::cout << GB2->getLocation() << std::endl;
  std::cout << GB2->getFunctionality() << std::endl;

  delete GB1;
  delete GB2;

  return 0;
}

[Ancient Dragon]

Possibly line 11 of main.cpp -- std: is a label, you want std:: (two colons, not just one).

The three set functions in the header file should be void functions because they do not return strings. Your compiler should be giving you warnings about that in the implementation *.cpp file., something like "function must return a value"

[mike_2000_17]

AD's right. The problem is probably with the return values of the set functions. I don't see anything else that could be wrong. Basically, if you don't return a value, then that creates an uninitialized value. This is simply because the memory for the return value is normally pre-allocated before entering the function, and then the function fills that memory with the return value (i.e. it is constructed in-place). If you have a function declared with a return type and then call it without assigning the return value to anything, it will just create a temporary variable for the return value, and that temporary will be immediately destroyed after coming back from the function call. If, on top of that, you don't have a return-statement in your function, then when you get back from the called function, the temporary variable is uninitialized and set for destruction, which is likely to crash (but often not, which explains the 4 successes and 1 failure).

So, do as AD suggests, make the return-type void for the set-functions. Also, you should always compile your code with the highest level of warnings (in GCC: -Wall, in MSVC: /W4), because an error like this would be caught, GCC would say "warning: control reaches end of non-void function." which is the kind of message that would have saved you a lot of time looking / asking around.

[benclifford]

Hi,
Thanks for the quick replies. I have now changed the set functions types to be void and they are now working.

Thanks.