What is wrong with my code? C++ and segmentation fault

mesh2005 · 07-22-2007, 06:27 AM

I'm trying to allocate an array of pointers dynamically. The array is allocated successfully in the routine "allocSpace" but when it returns to the main functions it is no longer available, it gives me 'Segmentation Fault'. Please help.

Code:

#include <iostream>
using namespace std;

typedef struct {
 char* name;   /* '\0'-terminated C string */
 int   num;
} SomeStruct;

void allocSpace(SomeStruct **);

int main ()
{

  SomeStruct **arr;
  allocSpace(arr);  
  for (int i=0;i<5;i++)
    cout<<arr[i]->num;
  return 0;
}

void allocSpace(SomeStruct **x)
{  
  x = (SomeStruct**) malloc(sizeof(SomeStruct *) * 5);
  for (int i=0 ; i<5 ; i++)
  {
        x[i] =  new SomeStruct;
        x[i]->num = i;
        cout<<x[i]->num;
  }  
}

ineya · 07-22-2007, 07:51 AM

arr is passed by value.

you can fix by adding reference (little confusing when near stars) or by adding one more star :-)

Code:

#include <iostream>
using namespace std;

typedef struct {
 char* name;   /* '\0'-terminated C string */
 int   num;
} SomeStruct;

void allocSpace(SomeStruct ***);

int main ()
{

  SomeStruct **arr;
  allocSpace(&arr);  
  for (int i=0;i<5;i++)
    cout<<arr[i]->num;
  return 0;
}

void allocSpace(SomeStruct ***x)
{  
  *x = (SomeStruct**) malloc(sizeof(SomeStruct *) * 5);
  for (int i=0 ; i<5 ; i++)
  {
        (*x)[i] =  new SomeStruct;
        (*x)[i]->num = i;
        cout<<(*x)[i]->num;
  }  
}

I also suggest using some typedefs.

wjevans_7d1@yahoo.co · 07-22-2007, 08:02 AM

When you call allocSpace(), parameter x is a local copy (within allocspace()) of variable arr in function main(). allocspace() alters that local copy when it does a malloc(). Variable arr in main() is never actually set or altered by your program.

You want arr to be an array of pointers to structs, with the array dynamically allocated with malloc() and the pointers dynamically allocated with new, right?

I present two solutions: an adequate one, and a better one.

The adequate one:

Code:

#include <iostream>
using namespace std;

typedef struct {
 char* name;   /* '\0'-terminated C string */
 int   num;
} SomeStruct;

void allocSpace(SomeStruct ***);

int main ()
{

  SomeStruct **arr;
  allocSpace(&arr);  
  for (int i=0;i<5;i++)
    cout<<arr[i]->num;
  return 0;
}

void allocSpace(SomeStruct ***x)
{  
  *x = (SomeStruct**) malloc(sizeof(SomeStruct *) * 5);
  for (int i=0 ; i<5 ; i++)
  {
        (*x)[i] =  new SomeStruct;
        (*x)[i]->num = i;
        cout<<(*x)[i]->num;
  }  
}

I think of this one as better for stylistic reasons:

Code:

#include <iostream>
using namespace std;

typedef struct {
 char* name;   /* '\0'-terminated C string */
 int   num;
} SomeStruct;

SomeStruct **allocSpace(void);

int main ()
{

  SomeStruct **arr;
  arr=allocSpace();
  for (int i=0;i<5;i++)
    cout<<arr[i]->num;
  return 0;
}

SomeStruct **allocSpace(void)
{  
  SomeStruct **result;

  result = (SomeStruct**) malloc(sizeof(SomeStruct *) * 5);
  for (int i=0 ; i<5 ; i++)
  {
        result[i] =  new SomeStruct;
        result[i]->num = i;
        cout<<result[i]->num;
  }  

  return result;
}

To make the code even simpler, consider the possiblity of allocating an array of SomeStruct directly, rather than allocating an array of individual pointers to SomeStruct.

Hope this helps.

Hivemind · 07-22-2007, 11:33 AM

I think you should decide if you want to learn C or C++. Your program, while it may compile as C++ code (it won't compile as C), is horrible mish-mash of both. A good C++ programmer knows where C and C++ differs and can handle raw arrays with confidence, but a good C++ programmer also puts what C++ offers to use too. And when you start learning C++, you should use std::string and std::vector, for example, from the start. These things were incorporated into C++ to assist the programmer and if you master the c++ standard library you've come far into the language.

wjevans_7d1@yahoo.co · 07-23-2007, 06:37 AM

Quoth hivemind:

Quote:

I think you should decide if you want to learn C or C++.

An excellent viewpoint, valid for many. It doesn't necessarily apply to everyone.

I use C mostly. C++'s fine additions can be used for a complete paradigm shift, as hivemind suggests. But they can also be used piecemeal in what would otherwise be a C program. That's the way I use C++.

I'm sure I'm passing up many aids to productivity by using C++ in such an ugly manner, but my method works for me, quickly and easily.

Be a purist if you want. That has many rewards, many of which are quite practical.

Or just grab those elements of C++ which float your boat, and go with it.

dmail · 07-23-2007, 08:14 AM

Unless you know what your doing and anybody using your code knows what they are doing do not mix malloc and new calls, they are not the same yet this instance makes no difference besides when freeing memory.

Personally I would have wrote it like so:

Code:

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


struct SomeStruct
{
explict SomeStruct(int const& number):m_number(number){}
SomeStruct(SomeStruct const& rhs):m_number(rhs.m_number),m_name(rhs.m_name){}
SomeStruct& operator=(SomeStruct const& rhs)
{
	m_number = rhs.m_number;
	m_name = rhs.m_name;
	return *this;
}
 int   m_number;
 std::string m_name;
};

typedef std::vector<SomeStruct>::iterator iter;


int main ()
{

std::vector<SomeStruct> arr;
//construct the objects
  for (int i=0 ; i<5 ; ++i)arr.push_back( SomeStruct(i) );
//print the values
  for (iter i=arr.begin() ;i != arr.end();++i)std::cout <<i->m_number <<std::endl;
}

wjevans_7d1@yahoo.co · 07-23-2007, 09:33 AM

Quoth dmail:

Quote:

Unless you know what your doing and anybody using your code knows what they are doing do not mix malloc and new calls

This is a good rule, but it is not as restrictive as it sounds. If:

neither you nor anybody maintaining your code follows a malloc()/realloc() by a delete or delete[] of the same object, and
neither you nor anybody maintaining your code follows a new with a free() of the same object,

then you can be considered to be "knowing what you are doing".

dmail · 07-23-2007, 09:53 AM

Quote:

This is a good rule, but it is not as restrictive as it sounds. If:...then you can be considered to be "knowing what you are doing".

This is untrue, as I have said new and malloc are not the same; if you know the differences and hide them then you can be considered to know what you are doing. There should be a very very good reason to use malloc in C++ if there is not then problems will follow.

Hivemind · 07-23-2007, 02:48 PM

malloc doesn't know about constructors, it allocates raw memory. If you're programming in C++ you use new and delete, if you program in C you use malloc (and its friends) and free.

This usenet post sums it up quite nicely:

Quote:

Reasons to always use new:

1. Somewhat friendlier syntax, as malloc() requires a cast unless the
results are assigned to a void*, and you usually do some arithmetic in
the size calculation. Examples:

int* arr = static_cast<int*>(malloc(5 * sizeof *arr));

int* arr = new int[5];

2. It gives a more consistent allocating scheme, so that you don't have
a mix of new and malloc(). Of course, one of the pitfalls of C++ is the
difference between new and new[], requiring delete or delete[]. There
is only one deallocation for malloc().

3. If you change your mind on a previously POD-type struct, you won't
have to find and change the malloc() calls.

exvor · 07-24-2007, 07:55 PM

Quote:

malloc() requires a cast unless the results are assigned to a void*

This is actually incorrect.

void pointers can become any kind of pointer. But I do agree that you should use new instead of malloc in C++

dmail · 07-25-2007, 04:55 AM

Quote:

Originally Posted by exvor

This is actually incorrect. ...

You are correct for C yet not C++.
http://www.research.att.com/~bs/bs_faq2.html#void-ptr

Hivemind · 07-25-2007, 05:32 AM

dmail beat me to it.

This is one of the cases where C and C++ differs. In C++, there are no implicit conversions from pointer-to-void to, say, pointer-to-char, a cast is required. In C it's not only unnecessary to cast what malloc returns it's bad because it can mask a failure to include its header. Anyway, this has been covered so many times and is getting a bit off-topic for the thread.

exvor · 07-25-2007, 02:42 PM

Silly C++

Yeah it is getting off topic sorry :P