segmentation fault with pointer in for loop

Question

The code compiles fine and runs fine until the for loop to iterate through f_read_Prediction_Set. The segmentation fault occurs directly after that line...what am I missing?

std::map< RAddr, uint32_t >* transCoherence::getCurrentSets(uint32_t log2AddrLs, uint32_t maskSets, uint32_t log2Assoc, int pid, RAddr caddr)//function to return prediction set
{
   uint32_t set;
   std::map< RAddr, uint32_t >* currentSets = new std::map< RAddr, uint32_t >;
   std::map< RAddr, uint32_t >* f_read_Prediction_Set = new std::map< RAddr, uint32_t >;

   for(std::map<RAddr, uint32_t>::iterator it = f_read_Prediction_Set->begin(); it!=f_read_Prediction_Set->end(); ++it)
   {
      set = (((it->first) >> log2AddrLs) & maskSets) << log2Assoc;
      if(set == caddr)
         (*currentSets)[set] = 1;
   }

   return currentSets;
}

@Beta - I was thinking that - Perhaps a demo on codepad would help — Ed Heal
– Ed Heal, Commented Feb 27, 2014 at 0:38
@Beta lol yes quickly, originally that line of code initializing the new map wasnt there and it wasnt empty and i got the same fault... — JazzyBelle
– JazzyBelle, Commented Feb 27, 2014 at 0:41
You have a memory leak in that function. Are you a former (or current) Java programmer, by any chance? — PaulMcKenzie
– PaulMcKenzie, Commented Feb 27, 2014 at 0:46

sh03 · Accepted Answer · 2014-02-27 00:55:27Z

1

Let me take your code full of pointers and dynamically allocation and translate it to an error and memory leak free version:

std::map<RAddr, uint32_t> transCoherence::getCurrentSets(uint32_t log2AddrLs, uint32_t maskSets, uint32_t log2Assoc, int pid, RAddr caddr) 
{
    std::map<RAddr, uint32_t> currentSets;
    std::map<RAddr, uint32_t> f_read_Prediction_Set;

    // populate f_read_Prediction_Set

    for (const auto& p : f_read_Prediction_Set) {
        uint32_t set = (((p.first >> log2AddrLs) & maskSets) << log2Assoc);
        if(set == caddr)
            currentSets[set] = 1;
    }

    return currentSets;
}

so that you can forget about the pointer problem entirely.

If you worried about performance you should note that there's a little thing in C++ called RVO (Return Value Optimization) that will avoid actually copying the vector back when the function returns.

Also I can't fail to notice that you use the word "set" in your map object's names, so it's good for you to know that there's an std::set class for you to use if you desire unique keys.

edited Feb 27, 2014 at 0:55

answered Feb 27, 2014 at 0:49

sh03

76.5k39 gold badges177 silver badges282 bronze badges

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

6 Comments

Teivaz Over a year ago

Heh, it looks much better now. But anyways it doesn't explain the segfault.

Jonathan Leffler Over a year ago

Now you just need to add some data to the f_read_Prediction_Set so that the returned map is not empty. :D

JazzyBelle Over a year ago

Thanks! does anyone know why that was happening though? My professor has guided me to use the pointers so I would like to be able to understand why my use of them is fail...also no im not a java programmer :p im not a programmer at all really im an EE with a thesis in computer arch, thanks for all the help :) (adds data to f_read_Prediction_Set)

sh03 Over a year ago

@JazzyBelle, Well, your professor is wrong. Pointers are disappearing very fast from the C++ scene and in C++14 there's little use for them. When you use new you always have to pair it with a delete. In your code the first mistake is that you are not deleting f_read_Prediction_Set by the end of the function (effectively leaking its memory). The second mistake is that you are also returning the pointer of the other dynamically allocated map to the callee who does not know if he has to delete it or not (possibly leaking that too).

sh03 Over a year ago

@JazzyBelle, as far as your segmentation goes: at first sight your code is fine. The segmentation fault might be happening somewhere else. But to be honest I've lost my patience with pointers months ago and I don't have the strength necessary to survive another pointer debugging (:P). The general rule of thumb is: never use naked pointers and if you really do use std::unique_ptr or std::shared_ptr and wrap them.

|

Collectives™ on Stack Overflow

segmentation fault with pointer in for loop

1 Answer 1

6 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

6 Comments

Your Answer

Sign up or log in

Post as a guest

Related