Iterator in for looping infinitely through list

Question

I'm using an iterator in a for loop, and for some reason it is looping infinitely. It appears to be doing what it is supposed to be doing, though, just infinitely.

Looking at other similar questions did not help me solve my issue, as I am not using splice anywhere and tried many different ways of implementing this loop (for, while, using an int for the conditional loop, using the iterator itself, iterator advance, etc). The most progress I've had was getting an invalid read instead (see below).

I double checked the code to see if I wasn't manipulating iterator pointers directly (possibly generating a situation where list.end() = list.begin()) but couldn't find anything of the sort. All I used for this specific list was find, insert and clear.

I'm iterating through a list of pairs composed of struct pointer REG and an int. This is compiled in C++11 with -Wall and -Wextra.

Modifying it with anything to control it to stop at the last item (using .size(), or checking to see if the same position of vector colors[] is already 1), results in an invalid read error.

int getAvailableColor(int K, std::list<std::pair<REG*, int>> adjacency)
{
   std::list<std::pair<REG*, int>>::iterator it;   
   int Ko = K;                                     
   int colors[K];                                  

   for(int i = 0; i < Ko; i++)
   {
       colors[i] = -1;
   }

   // This loop either breaks or runs forever... In some iterations
   // At this point K is as was passed as parameter
   for(auto it : adjacency)
   {
        if(it.second == 0){
            if(it.first->COLOR != -1)
            {
                colors[it.first->COLOR] = 1;
            }
        }
    }
    // At this point K is decreased

    for(int i = 0; i < Ko; i++)
    {
        if(colors[i] == -1) { return i; }
    }

    return -1;
}

Apparently, removing the array modification

colors[(*it).first->COLOR] = 1;

undoes the infinite loop issue (unfortunaly undoing the algorithm as well, so not a solution). Also, it seems this line is somehow decreasing the integer K.

The function call is done here:

    aux->COLOR = getAvailableColor(G.K, aux->ADJC);

Where aux is a REG* and ADJC is a list(REG*, int). The REG structure is:

typedef struct REG{
    int ID;
    int TYPE; // 0 - Physical, 1 - Virtual
    int COLOR;
    int CONN;
    std::list<INTERF> INTRFR;
    std::list<std::pair<REG*, int>> ADJC;

    bool operator > (const REG& reg) const { return (CONN > reg.CONN);        }
    bool operator < (const REG& reg) const { return (CONN < reg.CONN); }
    bool operator == (const REG& reg) const { return (ID == reg.ID); }
}REG;

INTERF typedef is std::pair(int, int), and the function that builds the list is:

void setAdjacency(GRAPH g)
{
    std::list<REG*>::iterator it;       
    std::list<INTERF>::iterator it2;     
    std::list<REG*>::iterator it3 ;      
    REG* aux = newReg(-1, -1, " ");
    REG* aux2;                           
    int count;

    for(it = g.LOGREG.begin(); it != g.LOGREG.end(); it++)
    {
        count = 0;
        (*it)->ADJC.clear();

        for(it2 = (*it)->INTRFR.begin(); it2 != (*it)->INTRFR.end(); it2++)
        {
            count++;
            aux2 = getRegister(g.REGISTERS, it2->first);

           (*it)->ADJC.insert((*it)->ADJC.end(), std::pair<REG*, int>(aux2, 0));
        }

        (*it)->CONN = count;
    }
}

When I stop the infinite loop this is the valgrind output:

==13369== Process terminating with default action of signal 2 (SIGINT)
==13369==    at 0x10D008: __gnu_cxx::__aligned_membuf<std::pair<REG*, int> >::_M_ptr()
==13369==    by 0x10C67B: std::_List_node<std::pair<REG*, int> >::_M_valptr()
==13369==    by 0x10BD18: std::_List_iterator<std::pair<REG*, int> >::operator*() const
==13369==    by 0x10B116: getAvailableColor(int, std::__cxx11::list<std::pair<REG*, int>, std::allocator<std::pair<REG*, int> > >)

And the invalid read error:

==13520== Invalid read of size 8
==13520==    at 0x10BFE3: std::_List_iterator<std::pair<REG*, int> >::operator++()
==13520==    by 0x10B159: getAvailableColor(int, std::__cxx11::list<std::pair<REG*, int>, std::allocator<std::pair<REG*, int> > >) 
==13520==    by 0x10AE10: runStack(GRAPH) 
==13520==    by 0x10A500: algorithm(GRAPH)
==13520==    by 0x1107AC: main 
==13520==  Address 0x10520fb40 is not stack'd, malloc'd or (recently) free'd
==13520== 
==13520== 
==13520== Process terminating with default action of signal 11 (SIGSEGV): dumping core
==13520==  Access not within mapped region at address 0x10520FB40
==13520==    at 0x10BFE3: std::_List_iterator<std::pair<REG*, int> >::operator++()
==13520==    by 0x10B159: getAvailableColor(int, std::__cxx11::list<std::pair<REG*, int>, std::allocator<std::pair<REG*, int> > >)

I would appreciate any help! This is my first real try at C++ and I'm sure there's much I don't understand about the structures.

EDIT: Using auto as was suggested seems to have at the very least stopped the infinite loop in the first test. As I did this I also noticed the value for K was changing between before the loop and after. Making a "backup" variable solved it. However, the invalid read error and the infinite loop still happen in some tests (same structure just different inputs). Updating code above accordingly.

EDIT2 (Solved): The infinite loop was solved substituting the array for a vector, and the invalid read error was an implementation logic issue (K was decremented at each iteration, so even though originally COLOR was only as big as K-1, with each iteration there as a bigger overflow). Thank you so much, everyone :)

Answer 1

The code concerning the iterator doesn't look especially wrong.

Make sure that you aren't accessing the array out of bounds at this instruction:
colors[(*it).first->COLOR] = 1;
This is the most probable cause of your problem.

However, since you aren't modifying the iterator, and don't access it outside of the loop, I would advice you to use the enhanced for loop syntax. Instead of: for(it = adjacency.begin(); it != adjacency.end(); it++)
You could easily replace with:
for (auto it: adjacency)
Unless you must be C++03 compatible. This will save you from a number of stupid errors.