Ya know, ROM already has a largely unused validation system. Grep around for IS_VALID.

You could probably setup a loop to call a function to grab the next iterator.

for( ch = ch_list ; ch ; ch = get_next(ch) )…

CHAR_DATA *get_next(CHAR_DATA *ch)
{     if( IS_VALID(ch->next ) )
            return ch->next;
      return NULL;
}

What if ch->next is invalid but ch->next->next is valid?

Well, in my example that would trigger end-of-loop. This could be undesirable in some cases, but surely it's better then a total bomb.

There is of course, constructing a temporary list, and iterating that instead of the main list. This way, when things are invalided and ignored, it can simply skip over it (the other way, this behaviour is not possible as the ch->next pointers is set to char_free).

Well, no, you could do it with a global char_next. It's not pretty, and only allows one iteration, but basically when you're about to delete a character if you detect that it's char_next, you update char_next.

Ya, definitely not pretty. Especially if you wanted to run through more then one loop at a time. Not to mention the fact that they exist in multiple lists.

'Well, no, you could do it with a global char_next. It's not pretty, and only allows one iteration, but basically when you're about to delete a character if you detect that it's char_next, you update char_next. '
Exactly what I am doing btw, in the extract char method, I test for all the iterator I use, basically I have seven loops in these case, where the char could be extracted without a way to notice the original loop.
I also made so extract_char return either the next valid char in room or in list depending on where you call it, it cuts the number of global iterator I have.(2 per loops)
It is ugly as it is not a generic way of doing thing you can do without thinking of it, but I did not find any pattern for a safe iterator in C like there is in more advanced programs.

Probably just a coincidence, but one of the immortals on the mud I code for was named Davion as well (Arcane Nites).

(Off Topic)

Every time I see the name Davion I have to wonder if the person in question is a BattleTech player. :/
House Davion of the Federated Suns… mmm… yeah. FedSuns is my favorite faction.

(On Topic)

I think you've pretty much discovered at this point that you need to implement some sort of delayed extraction. Its really the only safe way.

'I think you've pretty much discovered at this point that you need to implement some sort of delayed extraction. Its really the only safe way. '
The only 'coherent' way I guess. Global iterator works fine, just really ugly as you need twice as many than you have loops that could be modified while iterating them.

I hoped for a safe iterator code pattern in C that I could shoe in ROM code.
Guess not a lot of people use this piece of shit code, but I am way too lazy to migrate now that I basically fixed all the problems ROM had heh. Had I knew how bad it was when I began I would have swapped to another engine fromthe beginning :p (I tested CoffeeMud, though its code is not that clean, at least manipulating string is not a nightmare ;p)

I dunno, I've really enjoyed using ROM with C++ ehancements. I'm sure Java strings are easier to play with than C++, but the difference
beteween C style strings and C++ is a huge leap. Right now, I'm writing a string library for standard operations that the mud would need, such as
oneArg, isName, isPrefix, etc. Adding commands is pretty sweet now. All I have to do is pass the argument in the string constructor and do whatever I want
with it.

The downside though, is I've noticed that is annoying, is string iterators aren't really strings themeselves. So if you want to perform any operation, you have to
copy the value to a string itself, eg:

for ( string::iterator ch = str.begin(); ch != str.end(); c++ ) {
   if (*ch == "A") // error

   s = *ch;
   if (s == "A") // works
}

Or, you could just compare characters to characters?

for ( string::iterator ch = str.begin(); ch != str.end(); c++ ) {
   if (*ch == 'A')
      do_stuff();

   if(ch == "foo")
      do_other_stuff(); // Not sure if this would work, but it would make sense.
}

You're right, I forgot that a string::iterator returns a character not a string.
What was throwing me off was the error: ISO C++ forbids comparison between a pointer and integer

Ya know, I'm having a hard time trying to recreate this in a small test environment! I think what's happening, is, even though I'm freeing ch->next, it's still getting properly removed, and the next pointer is updated in free_char.

See, I was going to show that you do not need a delayed removal, but can use a duplicate of the list instead (it's a technique I picked up from Python, though, it's a helluva lot easier to dup a list in that ;))

However, I cannot break the original code. What am I doing wrong (right?!)?

[link=paste]37862[/link]



Edit:

I actually got it to break, by storing the next pointer.

int main( void )
{       CHAR_DATA *ch, *next;
        char_list = char_free = NULL;

        new_char("Davion");
        new_char("Ralgith");
        new_char("Quixadhaul");
        new_char("Jonny");

        for(ch = char_list ; ch ; ch = next )
        {       printf("%s\r\n", ch->name);
                next = ch->next;
                //blow stuff up
                if(ch->next && !strcasecmp(ch->next->name, "Ralgith") )
                free_char(ch->next);
        }

        return -1;
}

That will include an invalid node in the list.

The first think I did in Murk++ was remove the memory system and convert to std::list containers. It uncovered a whole host of bugs. The problem is not only is ch->next invalidated but very often is ch->next->next. This isn't just limited to the global list of characters, but also to the in_room lists of characters. Merc 2.2 mobprogs compounds the problem, because the code in triggers begins using room lists again which creates a nice nested mess of spaghetti. There are several viable solutions, none of which I implemented in Murk++; like creating and using a list container class with safe iterators, or marking mobiles and characters as "to be removed", they stay in the lists and are skipped during list processing and are removed later the main loops input/output cycle. Instead in Murk++, I defined a number of global iterators as a stupid hack around several of the problems, which doesn't solve all the problems. Maybe I'll work on really solving the problem and releasing a new version later this year.

Ya, it seems like you have to not only worry about ch->next being invalid, but ch, and ch->next->next. Odd ;).

In theory, you could have an infinite number of ->next's that could be invalidated.

For example, you have 20 characters in a room, the mob they're fighting bombs 17 of them out with an AOE spell. Now you've got 17 invalid positions in a 20 position list. Oops.

Unless you're ready to strip the whole system out and implement something else, the easy way to fix it is to use delayed extraction.

I updated my little test script

It uses a duplicate list over top of the original to protect your nodes. You can modify the original at will, because you are never actually iterating through the original, only a duplicate. The downside of this, is of course, double iteration.

By the way my solution that protects the iterator in case someone is interested ;p

CHAR_DATA *viol_upd_it;
CHAR_DATA *viol_upd_it_next;

void violence_update(void) {
CHAR_DATA *victim;

roundNumber++;
roundNumber = roundNumber % PULSE_TICK; //part of my solution to prevent player/mob assisting to have 2 multihits in the same round ;p
char buf[MAX_STRING_LENGTH];
for (viol_upd_it = char_list;viol_upd_it != NULL;viol_upd_it = viol_upd_it_next) {
viol_upd_it_next = viol_upd_it->next;
//yadayada, always testing for NULL if I use viol_upd_it after a command that could nulllify it)
}

now in extract char
//extract char returns the next valid char in the room or in the list, depeding on the one you want
CHAR_DATA *extract_char(CHAR_DATA *ch, const sh_int charReturnedType)

CHAR_DATA *nextValidChar = ch->next; //storing next valid chars (or NULL if it is the end of loop)
CHAR_DATA *nextValidRoomChar = ch->next_in_room;


//there before freeing the char, updating my iterators
if (ch == viol_upd_it) {
viol_upd_it = NULL;
viol_upd_it_next = nextValidChar;
}
if (ch == viol_upd_it_next)
viol_upd_it_next = nextValidChar;


Off course like I said, you have to do this for all the loops that can be modified while being iterated. I did this cause I only have 7 of them, thanksfully.

The best solution is the delayed extraction I think.
The solution I implemented as a quick fix has at least a temporary advantage:
It will allow me to check for bugs, because if I forget to check for NULL somewhere it will crash with a core dump.

Invalidating char has this tiny problem that if you use the char without testing before if it is invalidated, weird behaviour can happen that may be difficult to identify.

Just for the sake of it I started implemented delayed extraction to have a proper engine heh….
For a little info I have 64 loops over char_list….
And that is only for the loops, then you have to put checks for all method that can extract char and that have code after them..
damage being the main one with 170 occurence (again my mud) , you have an idea about how many methods you have to modify to clean a ROM….
Oh….and this is only about chars…objects could have the same problems, but I did not identify the chain reaction that could lead to it yet.

Like I was saying…not difficult to code…..just a hell lot of missing sleep time.

Ok I think I did it. To avoid to break any loops and ease death managing here is what I do:
In extracting char method, I just replace the the char by a empty char I mark invalid.
The char that was extracted is placed to the beginning of the char_list loop (in case you do not want to remove it from the mud, but just want to avoid to iterate over it before next updating loop
as an example:a char that died)

I had to check for every close_socket to either free char or extract it depending on y choice.

Of course, scrapped totally the memory recycling methods.

I free the memory myself after all the updates where done, in a loop checkig for char/mobs marked invalid.

So every method that use a char just have to test for null or ch->invalid

Dunno how other Codebase do it, but this is doable in ROM without too much an headache.