1

Well, I am writing a program for the university, where I have to put a data dump into HDF format. The data dump looks like this:

"1444394028","1","5339","M","873"
"1444394028","1","7045","V","0.34902"
"1444394028","1","7042","M","2"
"1444394028","1","7077","V","0.0470588"
"1444394028","1","5415","M","40"
"1444394028","1","7077","V","0.462745"
"1444394028","1","7076","B","10001101"
"1444394028","1","7074","M","19"
"1444394028","1","7142","M","16"
"1444394028","1","7141","V","0.866667"

For the HDF5 API I need an array. So my method at the moment is, to write the data dump into an array like this: 

int count = 0;
std::ifstream countInput("share/ObservationDump.txt");
std::string line;

if(!countInput) cout << "Datei nicht gefunden" << endl; 

while( std::getline( countInput, line ) ) {
    count++;
}

cout << count << endl;

struct_t writedata[count];

int i = 0;

std::ifstream dataInput("share/ObservationDump.txt");
std::string line2;

char delimeter(',');

std::string timestampTemp, millisecondsSinceStartTemp, deviceTemp, typeTemp, valueTemp;

while (std::getline(dataInput, timestampTemp, delimeter) ) 
{
    std::getline(dataInput, millisecondsSinceStartTemp, delimeter);
    std::getline(dataInput, deviceTemp, delimeter);
    std::getline(dataInput, typeTemp, delimeter);
    std::getline(dataInput, valueTemp);

    writedata[i].timestamp = atoi(timestampTemp.substr(1, timestampTemp.size()-2).c_str());
    writedata[i].millisecondsSinceStart = atoi(millisecondsSinceStartTemp.substr(1, millisecondsSinceStartTemp.size()-2).c_str());
    writedata[i].device = atoi(deviceTemp.substr(1, deviceTemp.size()-2).c_str());
    writedata[i].value = atof(valueTemp.substr(1, valueTemp.size()-2).c_str());
    writedata[i].type = *(typeTemp.substr(1, typeTemp.size()-2).c_str());

    i++; 
}

with struct_t defined as 

struct struct_t
{
    int timestamp;
    int millisecondsSinceStart;
    int device; 
    char type; 
    double value; 
};

As some of you might see, with big data dumps (at about 60 thousand lines) the array writedata tends to generate a stack overflow (segmentation error). I need an array to pass it to my HDF adapter. How can I prevent the overflow? I was not able to find answers by extensive googling. Thanks in advance!

Improve this question
14
  • 5
    struct_t writedata[count]; is a VLA and is not standard in C++. If you want a run time sized array use a std::vector. Commented May 8, 2017 at 13:15
  • Do you have a link to the documentation, or at least the signature, of the function you're using that requires an array? Commented May 8, 2017 at 13:21
  • A std::vector is probably better than reading through the file twice, but another option would be explicit new and delete (or malloc and free, but those are more C than C++). All three options (vector, new/delete, and malloc/free) put the data on the heap instead of the stack. Commented May 8, 2017 at 13:22
  • @dureuill Even for functions that require a C array, a vector works, because internally it stores its data contiguously. Commented May 8, 2017 at 13:23
  • @dureuill support.hdfgroup.org/ftp/HDF5/examples/examples-by-api/… is the documentation of how I built my adapter. Commented May 8, 2017 at 13:26

2 Answers 2

Reset to default
3

The example code you are following is in C, while the code you are writing is it C++. In most cases, valid C code is valid C++ code, although not necessarily good style; this is one of the times where it is not, although since that isn’t your real problem I’ll leave the explanation of that at the end of my answer.

When you declare struct_t writedata[count];, you are creating an array on the stack. The stack is often artificially limited in size, and so creating a large array on the stack could lead to a problem where you run out of stack space. This is what you are seeing. The typical solution is to create large data structures in the heap (although the primary use of the heap is to make data that lasts past the return of the function that creates it).

The most C++-idiomatic way to access the heap is to not do it directly, but to use a helper container class. In this case what you want is an std::vector, which lets you push data onto the end and will automatically grow as you push on more data. Since it automatically grows, you don’t need to specify the size in advance; just declare it as a std::vector<struct_t> writedata; (read “std::vector of struct_t”). Again, since it doesn’t need to know the size in advance, you can also ignore the whole first loop.

The vector is initially empty; to put data into it, you usually want to use writedata.push_back() or writedata.emplace_back(). The first of these takes an existing struct_t; the second takes the parameters you’d use to create one. All of the elements are stored contiguously in memory, like in a C array, which you can access directly with writedata.data().

At the end of the function, when the vector goes out of scope and is no longer accessible, its destructor will be called and automatically clean up the memory it used.

Another option, instead of using std::vector, is to manage the memory yourself. The C++ way of doing that is with new and delete. The easiest way to handle that is to still calculate count, as you do, but instead of creating the array on the stack by just declaring it as a count-sized array, you do struct_t* writedata = new struct_t[count];. This will create an array of count struct_ts in the heap, and set writedata as a pointer to the first element of this array. Then you can use it as you use the array in your program, but since it’s on the heap you won’t run out of stack space.

The downsides to this are that you need to know the size in advance, and you need to clean up the memory you used yourself. To do this, when you no longer need the data, you should run delete[] writedata. After that, writedata will still point to the same place in memory, but your program no longer owns that data, so you need to make sure to never use that value again; the standard way is to, immediately after deletion, set writedata to nullptr.

You can also use the C equivalents to new and delete, which are malloc and free. They are mostly equivalent in your case, but for more complicated examples you should keep in mind that these leave the memory uninitialized, while new and delete will run the constructors/destructors of what you create to make sure the objects are in a sane state at the beginning and don’t leave resources lying around at the end.

Now for why your original code isn’t actually valid C++ for any size of file: Your line struct_t writedata[count]; tries to create an array of count struct_ts. Since count is a variable, this is called a variable-length array (VLA). Such things are legal in newer versions of C, but not in C++. This alone is just worth a warning as long as you only want to compile the code on the same system you’re currently using, since your compiler seems to support VLAs as an extension. However, if you want to compile your code on any other system (make it more portable), you shouldn’t use compiler extensions like this.

Improve this answer
Sign up to request clarification or add additional context in comments.

9 Comments

"if you want to compile your code on any other system", including your own system in the future. Bottom line, unless you know very well what you're doing, write valid C++.
GCC’s VLA support is relatively stable. There’s a difference between taking advantage of undefined, undocumented behavior and taking advantage of an explicitly-documented extension. I don’t know what compiler the OP is using, but I would bet that since it supports VLAs at all it probably explicitly documents this somewhere. It’s still less stable than a feature of the language, but it probably won’t just disappear on a whim or to support some optimization nobody’s invented yet.
Yes there is a difference, you're correct and I agree, gcc probably won't throw VLA through the window anytime soon. Using non standard features remains a trade-off, though, and usually not a good one, since it may not be portable to other compiler and due to other standard features of C++ (eg, use a vector w/ a custom allocator if you really can't pay for heap allocation while pushing elements. Usually, you can pay, though.). I stay by my point that you shouldn't use "dialect" features unless you know what you're doing. Another extension where the tradeoff is better is #pragma once.
@dureuill That’s why I listed it as a warning at the end of the answer even though VLAs being non-standard wasn’t the issue here. Often there are good reasons for using compiler extensions instead of any of the standard features, but you should at least be aware that they are extensions and even if you don’t plan to change build systems you should think twice before using them.
Just to revisit these comments, I recently fixed a segfault in some C++ code that was caused by VLAs in gcc-4.9 (from jessie-backports) when compiling with the flag -std=c++14. Replacing the VLAs by std::vector fixed the bug. The bug did not occur with clang or different -std values. So much for "gcc probably won't throw VLA through the window anytime soon".
|
-1 
struct_t writedata[count];

This array is allocated on the stack which is normally quite small, and when it gets to a value that's too big (which is semi-arbitrary) this will overflow the stack. You'd be better off allocating on the heap by doing something like:

struct_t* writedata = (struct_t*)malloc(sizeof(struct_t) * count);

And then add a corresponding call to free once you're finished with the memory, e.g.

free(writedata);
writedata = nullptr;

It's best practice to check that i < count in your while loop, as if you write off the end of your array Bad Things may happen.

Improve this answer

13 Comments

You should not be using malloc. It does not call the constructor leaving all the elements in a uninitialized, undefined state.
There is no constructor in his struct
I would advocate std::vector over manual memory allocations. In any case this is c++, not c, you should be using new instead of malloc. Even then, prefer std::make_unique and std::make_shared over new for general cases.
@IainBrown Still a bad habit to get into. Just use new and you do not have to worry about it.
This is a C answer to a C++ question. One, one could argue this is C++ because of the nullptr, but still...
|

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.