Jump to content
  • Advertisement
Sign in to follow this  
D A

Questions about structs & dynamic memory

This topic is 4234 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

Lets say I have this code:
#include <iostream>
#include <string>

struct myStruct
{
   int foo;
   int bar;
   std::string someText;
};

void dataToStruct(myStruct *pStruct, const int n);
void printStruct(myStruct *pStruct, const int n);

int main()
{
   const int size = 10;
   myStruct *pStruct = new myStruct[size];
   dataToStruct(pStruct,size);
   printStruct(pStruct,size);
   return 0;
}

void void dataToStruct(myStruct *pStruct, const int n)
{
   for(int i = 0; i < n; i++)
   {
      pStruct->foo = 1200;
      pStruct->bar = 5953;
      pStruct->someText = "foobar";
      pStruct++;
   }
}

void printStruct(myStruct *pStruct, const int n)
{
   for(int i = 0; i < n; i++)
   {
      std::cout << pStruct->foo << "\n";
      std::cout << pStruct->bar << "\n";
      std::cout << pStruct->someText << "\n";
      pStruct++;
   }
}
1. How can I check if it was able to allocate all memory for my struct? 2. Where should I free the allocated memory? 3. What design changes should I make to this program? My programs gets it's struct variables from a file, and I cut that part out as it was quite a lot of space (this is not the important part of my questions). Thanks in advance

Share this post


Link to post
Share on other sites
Advertisement
Quote:

1. How can I check if it was able to allocate all memory for my struct?

This can be done in different ways, using exceptions, or even write your own new handler. But the simple way is to check if pStruct equals 0.

myStruct *pStruct = new myStruct[size];
if(pStruct == 0)
{
// Allocation failed. Probably due to an out of memory condition.
// You can check errno here to find the exact reason why it failed
// Report the error and quit...
}


Quote:

2. Where should I free the allocated memory?

In a simple application like this you should generally delete the memory in the same scope as you allocated it.
In this case it should happen in the scope of the main function.

int main()
{
const int size = 10;
myStruct *pStruct = new myStruct[size];
dataToStruct(pStruct,size);
printStruct(pStruct,size);
delete [] pStruct; // here...
return 0;
}

Quote:

3. What design changes should I make to this program?

Your dataToStruct and printStruct is probably not working the way you want.
Remember that the pStruct pointer is always pointing to the first element in the array, so in your loops:

...
// This operates on the first array element every iteration
// which is probably not what you want
pStruct->foo = 1200;
...

In order to step through the array in your loops you need something like

...
pStruct.foo = 1200;
...

This is more like an error though. Not a design problem =)

Share this post


Link to post
Share on other sites
Quote:
Original post by D A
Lets say I have this code:

1. How can I check if it was able to allocate all memory for my struct?


By default, new will throw an exception if it fails to allocate all memory.

Quote:

2. Where should I free the allocated memory?


As soon as you don't need it anymore, and preferably in the same place or object that allocated it in the first place (also referred to as its owner).

Quote:

3. What design changes should I make to this program?


Use std::vector instead of using a buffer and passing its size along, use print and fromData methods for single structures instead of applying them to entire buffers, and use std::for_each along with std::mem_fun_ref to call a member function on all elements in a vector.

Share this post


Link to post
Share on other sites
1) 'new' throws an exception if the requested memory can not be allocated for any reason. So the correct way of doing that in C++ is a try-catch block:

int main() {
try {

const int size = 10;
myStruct *pStruct = new myStruct[size];
dataToStruct(pStruct,size);
printStruct(pStruct,size);

} catch( const bad_alloc& ) {

// handle the situation here

}

return 0;
}


Having said that, adding a try-catch block for every single memory allocation request, clutters the code to a great extent and that's why people usually test the result against NULL or totally ignore that, although the latter might not be a good practice.

2) All variables are associated with scopes and because pointers are variables, they are associated with a scope too. You can delay memory deallocation as long as the pointer is in scope. If you delay memory deallocation beyond that point, the pointer will not be accessible. If that pointer is the only one holding a reference to the allocated memory, there will be no way to access that memory neither. Such a memory is said to be leaked.

In your case, the pointer pStruct will be out of scope when the main() block is over. So you must deallocate the memory any time up to that point or your allocated memory will not be accessible forever!


int main() {
try {

const int size = 10;
myStruct *pStruct = new myStruct[size];
dataToStruct(pStruct,size);
printStruct(pStruct,size);

delete[] pStruct;

} catch( const bad_alloc& ) {

// handle the situation here

}

return 0;
}

Share this post


Link to post
Share on other sites
C++'s new throws an exception if the memory can't be allocated. You can find out if it failed by 'catching' the exception:


int main()
{
const int size = 10;
try {
myStruct *pStruct = new myStruct[size]; // could throw an exception
dataToStruct(pStruct,size);
printStruct(pStruct,size);
delete[] pStruct;
}
catch (std::exception& e) {
std::cout << "Could not allocate memory!" << std::endl;
}

return 0;
}


You could improve the design by having dataToStruct and printStruct instead be functions of the myStruct struct, and by using a std::vector instead of allocating your own array. dataToStruct's operations could be done in the myStruct constructor:


int main()
{
const int size = 10;
try {
std::vector<myStruct> objects;
for (size_t i = 0; i < size; i++) {
objects.push_back( myStruct(1200, 5953, "foobar") );
}

for (size_t i = 0; i < size; i++) {
objects.print();
}
}
catch (std::exception& e) {
std::cout << "Something unfortunate happened!" << std::endl;
}

return 0;
}


Another advantage to the std::vector approach is not having to worry about memory allocation. Notice there's no delete, because we're not using pointers with the myStruct objects. Just letting std::vector deal with it. When the vector goes out of scope (at the end of the try block) it deletes everything contained in it.

(I don't know why the 'plus plus' on my for loops isn't showing up in preview. Obviously they should be there.)

Share this post


Link to post
Share on other sites
Quote:

Having said that, adding a try-catch block for every single memory allocation request, clutters the code to a great extent and that's why people usually test the result against NULL or totally ignore that, although the latter might not be a good practice.

Except since new throws by default, checking for null won't work, so that's not good practice either.

Wrapping all allocation requests in try/catch is pointless. This can be generalized: you should catch an exception from the point at which you are able to handle or recover from that exception, and no sooner.

Many times, a failed allocation indicates that the function cannot continue, but that that larger process in place might be able to be restarted or at least terminated gracefully.

Not only to unneccessary try/catch blocks clutter the code, they generally don't actually improve stability, and in fact they can make dealing with exceptions more difficult and reduce the overall stability of the code. Writing solid exception safe code isn't just a matter of wrapping everything operation that could throw in a try block.

Share this post


Link to post
Share on other sites
Quote:
Original post by D A
Lets say I have this code:

...

1. How can I check if it was able to allocate all memory for my struct?


The first respondent is incorrect; the others are (generally) correct.

'new', in modern C++, and without special hackery, will throw an exception if it fails (specifically, an instance of std::bad_alloc). If you don't catch it anywhere, that will just terminate the program (but in a way that unwinds the entire stack and properly destructs everything). This is usually a perfectly fine way of dealing with a problem of this sort (it's usually very difficult if not impossible to recover; note that std::bad_alloc could also be thrown if the heap is corrupted due to programmer error).

Quote:
2. Where should I free the allocated memory?


When you are done with it. In your case, at the end of main().

But please don't say "free" the allocated memory; that implies use of std::free(), which you should only use for memory allocated with std::malloc() (and in modern C++, without *extremely* strange things going on, you should never allocate memory that way anyway). To deallocate memory that was new'd, use delete. To deallocate memory that was new[]'d, use delete[]. Note the distinction: it is *required* that you match things up. No exceptions.

If you want to talk about deallocation of memory in general, then the general term is - as I've been using - deallocation. :)

Quote:
3. What design changes should I make to this program?


Changes which prevent you from dealing with the memory management explicitly. :)

As illustrated, the size is a compile-time constant; thus there's no reason to allocate the memory dynamically at all: just use "myStruct structs[size];" and pass "structs" to the functions (yes, the array name, by itself, is usable as a pointer: we say it "decays" to a pointer to element 0).

However, if the size is *not* known ahead of time, you can make your life easier by using std::vector. This way, you can simplify your interfaces by not passing around length counts; the vector tracks how many elements are in there. You can also (sometimes; if there are other things afterwards in the file, you need to know where the current set of data stops and the next starts) avoid having to write length counts in your data files (which in turn makes it easier to edit them), because you can just iteratively add elements to the vector and it will accomodate them as they are added (subject to limitations on the total available memory, of course).

Basically, it maintains an allocated space, and tracks both the "capacity" of that space and the "size" of the actual data, and when it runs out of room, it makes a new allocation of a larger capacity, copies everything across and deallocates the old storage. It does this very carefully with some definitely-not-for-beginners optimizations; please don't try to reinvent the wheel.

Oh, some other changes: Since a vector is an object, we can return it from the loading function to make things more intuitive (remember, the storage is hidden behind a pointer to allocated memory, so there are no problems on the stack); we can make the struct printable and constructible; and we can make use of standard library algorithms to neaten up our loops and so forth. Oh, and I prefer to put helper functions first, so as to avoid prototypes in implementation files (you'll still need them in header files when you expose an interface).


#include <iostream>
#include <string>
#include <vector>
#include <algorithm>
#include <iterator>

struct myStruct {
int foo;
int bar;
std::string someText;

myStruct(int foo, int bar, const std::string& someText) :
foo(foo), bar(bar), someText(someText) {}
};

std::ostream& operator<<(ostream& os, const myStruct& s) {
return os << s.foo << '\n' << s.bar << '\n' << s.someText;
}

std::vector<myStruct> create() {
myStruct sample(1200, 5953, "foobar");
// One of the std::vector constructors takes a count and a 'prototype',
// and initializes the vector with count-many copies of the prototype:
return std::vector<myStruct>(10, sample);
// Of course, you'll need more information than that for loading from a file :)
// So see here: http://www.sgi.com/tech/stl/Vector.html
}

void output(const std::vector<myStruct>& stuff) {
// This is the power of standard library algorithms at work :)
// We "copy" the contents of the vector "to the output".
// 'iterators' are things that specify positions in containers, roughly;
// the ostream_iterator is an adapter that lets you treat an ostream like
// a container into which things are output.
// We need the first two iterators to specify the portion of the vector
// that we're copying - all of it - and the vector is nice enough to supply
// appropriate iterator objects to us:
std::copy(stuff.begin(), stuff.end(),
std::ostream_iterator<myStruct>(std::cout, "\n"));
// EDIT: Thanks, ToohrVyk. Totally wasn't paying attention.
// See? Copy-and-paste is BAD kids! :)
}

int main() {
std::vector<myStruct> myStructs = create();
output(myStructs);
// Of course, we could have just done 'output(create());'
// or perhaps inlined everything, or all kinds of other interesting
// combinations :) There really isn't enough code here to say what's best.
}




(Zomg, it's almost like a whole other language. :) A language in which things tend to be shorter - once you take out all my teaching comments :) - and pretty simple to understand, due to everything being named nicely and memory-management gruntwork being taken care of in clever ways behind the scenes.)

[Edited by - Zahlman on November 17, 2006 4:17:37 PM]

Share this post


Link to post
Share on other sites
Quote:
Original post by jpetrie
Quote:

Having said that, adding a try-catch block for every single memory allocation request, clutters the code to a great extent and that's why people usually test the result against NULL or totally ignore that, although the latter might not be a good practice.

Except since new throws by default, checking for null won't work, so that's not good practice either.


I think he meant that people usually use the no-throw new and then check for NULL. Except (a) that's not true from what I've seen; and (b) doing so would clutter the code at least as much - you would think not as much because the if-check is more compact than setting up a try-catch and scoping the variable outside the try-catch, but in practice you end up adding huge amounts of error propagation code - exactly what exceptions are meant to clean up.

Of course, in C, you don't have the option, and checking for NULL is - at least in theory - a good idea, because otherwise your program would dereference the null pointer and enter undefined behaviour land. Which is a much, much less happy way of crashing (and if you're unlucky, it might limp on for a little while in wildly unpredictable ways) than what happens when you don't catch an exception (i.e. std::terminate()). It's a shame about all the extra code, though. Fortunately, relatively few people have to write C these days. ^__^v

Quote:
Writing solid exception safe code isn't just a matter of wrapping everything operation that could throw in a try block.


Quoted for emphasis.

Share this post


Link to post
Share on other sites
Zahlman: you have (at least) one error in your code.

std::ostream_iterator<int>(std::cout, "\n"));

You're outputting myStructs, not integers.

Share this post


Link to post
Share on other sites
Quote:

Except since new throws by default, checking for null won't work, so that's not good practice either.

This is not the entire truth (Interresting way to put it these days ehh?)
The nothrow operator news default behavior is to return 0 on failure.
In order to use the nothrow version of new the <new> header must be included first. (I admit I failed to get this right in my reply above)

Quote:

The first respondent is incorrect; the others are (generally) correct.

May I ask what is generally incorrect?

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

Participate in the game development conversation and more when you create an account on GameDev.net!

Sign me up!