I’m writing C++ code to deal with a bunch of histograms that are populated from laboratory measurements. I’m running into problems when I try to organize things better, and I think my problems come from mishandling pointers and/or structs.

My original design looked something like this:

// the following are member variables
Histogram *MassHistograms[3];
Histogram *MomentumHistograms[3];
Histogram *PositionHistograms[3];

where element 0 of each array corresponded to one laboratory measurement, element 1 of each corresponded to another, etc. I could access the individual histograms via MassHistograms[0] or similar, and that worked okay. However, the organization didn't seem right to me—if I were to perform a new measurement, I’d have to add an element to each of the histogram arrays. Instead, I came up with

struct Measurement {
    Histogram *MassHistogram;
    Histogram *MomentumHistogram;
    Histogram *PositionHistogram;
};

As an added layer of complexity, I further wanted to bundle these measurements according to the processing that has been done on their data, so I made

struct MeasurementSet {
    Measurement SignalMeasurement;
    Measurement BackgroundMeasurement;
};

I think this arrangement is much more logical and extensible—but it doesn’t work ;-) If I have code like

MeasurementSet ms;
Measurement m = ms.SignalMeasurement;
Histogram *h = m.MassHistogram;

and then try to do stuff with h, I get a segmentation fault. Since the analogous code was working fine before, I assume that I’m not properly handling the structs in my code. Specifically, do structs need to be initialized explicitly in any way? (The Histograms are provided by someone else’s library, and just declaring Histogram *SomeHistograms[4] sufficed to initialize them before.)

I appreciate the feedback. I’m decently familar with Python and Clojure, but my limited knowledge of C++ doesn’t extend to [what seems like] the arcana of the care and feeding of structs :-)

What I ended up doing

I turned Measurement into a full-blown class:

class Measurement {
    Measurement() {
        MassHistogram = new Histogram();
        MomentumHistogram = new Histogram();
        PositionHistogram = new Histogram();
    };

    ~Measurement() {
        delete MassHistogram;
        delete MomentumHistogram;
        delete PositionHistogram;
    };

    Histogram *MassHistogram;
    Histogram *MomentumHistogram;
    Histogram *PositionHistogram;
}

(The generic Histogram() constructor I call works fine.) The other problem I was having was solved by always passing Measurements by reference; otherwise,开发者_开发百科 the destructor would be called at the end of any function that received a Measurement and the next attempt to do something with one of the histograms would segfault.

Thank you all for your answers!

Are you sure that Histogram *SomeHistograms[4] initialized the data? How do you populate the Histogram structs?

The problem here is not the structs so much as the pointers that are tripping you up. When you do this: MeasurementSet ms; it declares an 'automatic variable' of type MeasurementSet. What it means is that all the memory for MeasurementSet is 'allocated' and ready to go. MeasurementSet, in turn, has two variables of type Measurement that are also 'allocated' and 'ready to go'. Measurement, in turn, has 3 variables of type Histogram * that are also 'allocated' and 'ready to go'... but wait! The type 'Histogram *' is a 'pointer'. That means it's an address - a 32 or 64 bit (or whatever bit) value that describes an actual memory location. And that's it. It's up to you to make it point to something - to put something at that location. Before it points to anything, it will have literally random data in it (or 0'd out data, or some special debug data, or something like that) - the point is that if you try to do something with it, you'll get a segmentation fault, because you will likely be attempting to read a part of data your program isn't supposed to be reading.

In c++, a struct is almost exactly the same thing as a class (which has a similar concept in python), and you typically allocate one like so:

m.MassHistogram = new Histogram();

...after that, the histogram is ready-to-go. However, YMMV: can you allocate one yourself? Or can you only get one from some library, maybe from a device reading, etc? Furthermore, although you can do what I wrote, it's not necessarily 'pretty'. A c++-ic solution would be to put the allocation in a constructor (like init in python) and delete in a destructor.

When your struct contains a pointer, you have to initialize that variable yourself. Example

struct foo
{
    int *value;
};

foo bar;
// bar.value so far is not initialized and points to a random piece of data

bar.value = new int(0);
// bar.value now points to a int with the value 0

// remember, that you have to delete everything that you new'd, once your done with it:
delete bar.value;

First, always remember that structs and classes are almost exactly the same things. The only difference is that struct members are public by default, and a class member is private by default. But all the rest is exactly the same.

Second, carefully differentiate between pointers and objects.

If I write

Histogram h;

space for histogram's data will be allocated, and it's constructor will be called. ( A construct is a method with exactly the same name as the class, here Historgram() )

If I write

Histogram* h;

I'm declaring a variable of 32/64 bits that will be used as a pointer to memory. It's initialzed with a random value. Dangerous!

If I write

Histogram* h = new Histogram();

memory will be allocated for one Histogram's data members, and it's constructor will be called. The address in memory will be stored in "h".

If I write

Histogram* copy = h;

I'm again declaring a 32/64 bit variable that points to exactly the same address in memory as h

If I write

Histogram* h = new Historgram;
Histogram* copy = h;
delete h;

the following happens

1. memory is allocated for a Histogram object
2. The constructor of Histogram will be called (even if you didn't write it, your compiler will generate one).
3. h will contain the memory address of this object
4. the delete operator will call the destructor of Histogram (even if you didn't write it, your compiler will generate one).
5. the memory allocated for the Histogram object will be deallocated
6. copy will still contain the memory address where the object used to be allocated. But you're not allowed to use it. It's called a "dangling pointer"
7. h's contents will be undefined

In short: the "n.MassHistogram" in your code is referring to a random area in memory. Don't use it. Either allocated it first using operator "new", or declare it as "Histogram" (object instead of pointer)

Welcome to CPP :D

You are aware that your definition of Measurement does not allocate memory for actual Histograms? In your code, m.MassHistogram is a dangling (uninitialized) pointer, it's not pointing to any measured Histogram, nor to any memory capable of storing a Histogram. As @Nari Rennlos posted just now, you need to point it to an existing (or newly allocated) Histogram.

What does your 3rd party library's interface look like? If it's at all possible, you should have a Measurement containing 3 Histograms (as opposed to 3 pointers to Histograms). That way when you create a Measurement or a MeasurementSet the corresponding Histograms will be created for you, and the same goes for destruction. If you still need a pointer, you can use the & operator:

struct Measurement2 {
    Histogram MassHistogram;
    Histogram MomentumHistogram;
    Histogram PositionHistogram;
};

MeasurementSet2 ms;
Histogram *h = &ms.SignalMeasurement.MassHistogram; //h valid as long as ms lives

Also note that as long as you're not working with pointers (or references), objects will be copied and assigned by value:

MeasurementSet ms;                    //6 uninitialized pointers to Histograms
Measurement m = ms.SignalMeasurement; //3 more pointers, values taken from first 3 above
Histogram *h = m.MassHistogram;       //one more pointer, same uninitialized value

Though if the pointers had been initialized, all 10 of them would be pointing to an actual Histogram at this point.

It gets worse if you have actual members instead of pointers:

MeasurementSet2 ms;                    //6 Histograms
Measurement2 m = ms.SignalMeasurement; //3 more Histograms, copies of first 3 above
Histogram h = m.MassHistogram;         //one more Histogram

h.firstPoint = 42;
m.MassHistogram.firstPoint = 43;
ms.SignalMeasurement.MassHistogram.firstPoint = 44;

...now you have 3 slightly different mass signal histograms, 2 pairs of identical momentum and position signal histograms, and a triplet of background histograms.