[Home] [Downloads] [Search] [Help/forum]

Gammon Software Solutions forum

See www.mushclient.com/spam for dealing with forum spam. Please read the MUSHclient FAQ!

[Folder]  Entire forum
-> [Folder]  Dawn of Time
. -> [Folder]  Administration
. . -> [Subject]  Two Questions: a) Code Still Supported? b) Crashing somewhere in show_score()
Home  |  Users  |  Search  |  FAQ
Username:
Register forum user name
Password:
Forgotten password?
(New message)
Subject: Two Questions: a) Code Still Supported? b) Crashing somewhere in show_score()
Name:
Your forum user name.
Register forum user name
Password:
Your forum password.
Forgotten password?
Message:
Message to be posted (in English, please)
Maximum of 6000 characters. Text only please, no HTML.
Forum codes:
Check this if your message uses 'forum codes' or templates (auto-detected for new posts).
Forum codes Templates

Save this message ...

Subject review (reverse sequence)

Pages: 1 2  

Posted by Parsival   (6 posts)  [Biography] bio
Date Wed 11 Feb 2009 04:15 AM (UTC)  [ quote  ]
Message
Thanks for all your help, friends.
S
[Go to top] top
Posted by David Haley   USA  (3,881 posts)  [Biography] bio   Moderator
Date Mon 09 Feb 2009 07:00 PM (UTC)  [ quote  ]

Amended on Mon 09 Feb 2009 07:01 PM (UTC) by David Haley

Message
Well I did say that it depended on the precedence (which I couldn't recall at the time), and that I was assuming that the assignment was evaluated as the expression test :-)
David Haley aka Ksilyan
Head Programmer,
Legends of the Darkstone

http://david.the-haleys.org
[Go to top] top
Posted by Nick Gammon   Australia  (18,770 posts)  [Biography] bio   Forum Administrator
Date Mon 09 Feb 2009 06:45 PM (UTC)  [ quote  ]
Message
Quote:

Because in the latter, wch gets the value from the conditional, whereas in the former, wch just gets levelsort and the whole expression evaluates to the value from the conditional.


I cannot agree with you there, David. You are right that the overall behaviour depends on the operator precedence, but the C++ operator precedence is well defined, with the ternary condition having a higher precedence than direct assignment. See, for example:

http://en.wikipedia.org/wiki/Operators_in_C_and_C%2B%2B

There are some subtleties, using the ternary operator, if you also use = inside the expression (after the ?) however that does not apply in this case.

This test program illustrates it.


#include <iostream>

int main (void)
  {
  int a, b;
  int c = 1;

  a = (b = c ? 4 : 5);

  std::cout << "a = " << a << std::endl;
  std::cout << "b = " << b << std::endl;
  std::cout << "c = " << c << std::endl;

  return 0;
  }


Output

a = 4
b = 4
c = 1


You will note that both a and b are the value 4, whereas you predicted that b would take on the value of c (which is 1).
- Nick Gammon

www.gammon.com.au, www.mushclient.com
[Go to top] top
Posted by David Haley   USA  (3,881 posts)  [Biography] bio   Moderator
Date Mon 09 Feb 2009 01:16 PM (UTC)  [ quote  ]

Amended on Mon 09 Feb 2009 01:18 PM (UTC) by David Haley

Message
It depends on the precedence -- if the assignment is evaluated as the expression test, that is different from assigning levelsort ? x : y to wch. I.e.,

(wch = levelsort ? wch->next_who_list : wch->next_player)

is not the same as

wch = (levelsort ? wch->next_who_list : wch->next_player)

Because in the latter, wch gets the value from the conditional, whereas in the former, wch just gets levelsort and the whole expression evaluates to the value from the conditional.

However,

wch = (wch = levelsort ? wch->next_who_list : wch->next_player)

is indeed the same thing as

wch = (levelsort ? wch->next_who_list : wch->next_player)

as far as the final value of wch is concerned.
David Haley aka Ksilyan
Head Programmer,
Legends of the Darkstone

http://david.the-haleys.org
[Go to top] top
Posted by ThomasWatts   USA  (66 posts)  [Biography] bio
Date Sun 08 Feb 2009 06:26 AM (UTC)  [ quote  ]
Message
Yes, perhaps it should be wch == levelsort
Check for comparison then assign.
[Go to top] top
Posted by Nick Gammon   Australia  (18,770 posts)  [Biography] bio   Forum Administrator
Date Sat 07 Feb 2009 09:44 PM (UTC)  [ quote  ]
Message
It still seems redundant to me.


(wch = levelsort ? wch->next_who_list : wch->next_player)


... establishes a new value for wch. Then assigning that to wch doesn't seem to me to achieve much.
- Nick Gammon

www.gammon.com.au, www.mushclient.com
[Go to top] top
Posted by ThomasWatts   USA  (66 posts)  [Biography] bio
Date Sat 07 Feb 2009 08:33 AM (UTC)  [ quote  ]
Message
I hate to barge into this so late in the game, but

for ( wch = first_on_who; wch; wch = levelsort?wch->next_who_list:wch->next_player )

is missing its parenthesis, which is why 'wch = wch' seemed redundant.
Should be

for ( wch = first_on_who; wch; wch = (wch = levelsort ? wch->next_who_list : wch->next_player) )


As for the 'timebuf', I'm not sure why there is three. Each time the function is called the string is changed. Nothing outside the function can use 'timebuf' either.
8192 is also going to be a waste of memory. 1024 would be more than enough string space.
[Go to top] top
Posted by Nick Gammon   Australia  (18,770 posts)  [Biography] bio   Forum Administrator
Date Fri 06 Feb 2009 07:42 PM (UTC)  [ quote  ]
Message
It is an ingenious suggestion, but the timebuf buffer is statically allocated fixed-length buffer. Thus, it is never reallocated at runtime.

I initially thought that maybe he used 3 indexes so he could compare times (eg. this time compared to last time) but I can't see anywhere that is done.


Quote:

Your kindness is noted and very much appreciated!


Thanks for that. :)
- Nick Gammon

www.gammon.com.au, www.mushclient.com
[Go to top] top
Posted by Parsival   (6 posts)  [Biography] bio
Date Fri 06 Feb 2009 12:22 PM (UTC)  [ quote  ]
Message
At your suggestion, in get_ictime() located in ictime.cpp, I changed the declaration of the array to read:


 static char timebuf[3][MSL];


It worked perfectly, and pointed out very clearly that I need to brush up on syntax after so many years of inactivity.

By the way, I think I may understand the purpose of that line 
++index%=3;
now.

Using the modulus operator in this way causes index to cycle through values of 0, 1, and 2, then starting again at 0.

As I think about why, I'm wondering if perhaps this is done to reuse the memory allocated to timebuf[][] repeatedly rather than reallocating every time the function is called?

In other words, the string is written to timebuf[0][...] the first time, then timebuf[1][...] the second, and timebuf[2][...] the third. Then it starts with timebuf[0][...] again, overwriting the old information stored there.

Players type 'score' quite often, I'd expect, so this may be a little faster than reallocating memory every time this function is called? Am I off base?

If this -is- the case, would a defined macro be even faster and more efficient for this process?

Thanks a million, Nick!

-S
[Go to top] top
Posted by Nick Gammon   Australia  (18,770 posts)  [Biography] bio   Forum Administrator
Date Fri 06 Feb 2009 02:50 AM (UTC)  [ quote  ]
Message
Quote:

What I don't understand is the line:

++index%=3;

What is the purpose of this? I did a 'print index' prior to this line, which indicated 0, and afterward it contained 1.
Why not just index=1;? It's a local integer used in an array later...


Well for one thing it is declared static, which means its value is preserved between calls.

What that line is doing is:


  • Add 1 to index
  • Take the modulus of that new value, modulo 3.
  • Store the result back into index.


Why, I am not sure.

Anyway, I can't help thinking the declaration is backwards. Take this:


static char timebuf[MSL][3];


MSL is declared in params.h to be 8192. So it is using a big buffer, and 3 of them (one for each possible index, 0, 1 and 2).

But it is being used the other way around, eg.


// initialise working string
    timebuf[index][0]=0;


Offhand I would try this:


static char timebuf[3][MSL];


Then at least the order in the declaration agrees with the usage.

- Nick Gammon

www.gammon.com.au, www.mushclient.com
[Go to top] top
Posted by Parsival   (6 posts)  [Biography] bio
Date Thu 05 Feb 2009 11:47 PM (UTC)  [ quote  ]
Message
Thanks! I corrected that bug, hopefully avoiding a future crash.

After your help with gdb, I was able to set some breakpoints and step through the code, and I'm confident I've found the function that is causing the problem.

In ictime.cpp, the function get_ictimediff() is called from show_score(), which is called from do_score().

This function is also called from do_rpsheet(), and when I type that command, the mud crashes in exactly the same way.

Here is the code for that function. From what I can understand, this function returns a string containing the age of the character in years, days, months and seconds, taking into account the current amount of time played and the starting age of a character (17 years).

It's been a -long- time, Nick, since I studied C++, and in my current vocation, I have no need for it at all, so my memory is fuzzy on the algorithm for deriving time from a number of seconds...but if I'm reading this right, the %= operator is used to distil the age of the character in seconds ( abs((int)(t1-t2)) ) down to years, days, months and seconds.

What I don't understand is the line:

++index%=3;

What is the purpose of this? I did a 'print index' prior to this line, which indicated 0, and afterward it contained 1.
Why not just index=1;? It's a local integer used in an array later...

Anyway the line where the mud crashes is the sprintf statement. I'm really fuzzy on multidimensional array syntax these days, but it seems strange to me how timebuf[] is created and used in this function.

Here is the function in question:

char *get_ictimediff(time_t t1, time_t t2, int icyears_added_to_result)
{
    static char timebuf[MSL][3];
    static int index; 
    int dsec, icyear, icmonth, icweek, icday;

    ++index%=3;

    // initialise working string
    timebuf[index][0]=0;

    dsec   = abs((int)(t1-t2)); // number of IRL seconds between the 2 times
	icyear = dsec/ICTIME_IRLSECS_PER_YEAR;
	icyear+= icyears_added_to_result;
	dsec  %= ICTIME_IRLSECS_PER_YEAR;
    icmonth= dsec/ICTIME_IRLSECS_PER_MONTH;
	dsec  %= ICTIME_IRLSECS_PER_MONTH;
    icweek = dsec/ICTIME_IRLSECS_PER_WEEK;
	dsec  %= ICTIME_IRLSECS_PER_WEEK;
	icday  = dsec/ICTIME_IRLSECS_PER_DAY;	

    sprintf(timebuf[index],"%d year%s, %d month%s, %d week%s, %d day%s",
		icyear, 
		(icyear!=1?"s":""),
		icmonth, 
		(icmonth!=1?"s":""),
		icweek, 
		(icweek!=1?"s":""),
		icday, 
		(icday!=1?"s":""));

    return(timebuf[index]);
}


Thanks again for your time, Nick. I promise to dust off those old C and C++ books and make you proud one day....

S
[Go to top] top
Posted by Nick Gammon   Australia  (18,770 posts)  [Biography] bio   Forum Administrator
Date Thu 05 Feb 2009 02:31 AM (UTC)  [ quote  ]
Message
I changed that line to:


for ( wch = first_on_who; wch; wch = levelsort?wch->next_who_list:wch->next_player )


In other words, I got rid of the redundant assignment. It didn't crash next time, but then it didn't crash every time anyway, so I'm not sure that is really the problem.
- Nick Gammon

www.gammon.com.au, www.mushclient.com
[Go to top] top
Posted by Nick Gammon   Australia  (18,770 posts)  [Biography] bio   Forum Administrator
Date Thu 05 Feb 2009 02:29 AM (UTC)  [ quote  ]
Message
Well this is an interesting challenge. I can't get it to crash consistently (itself a bad thing) but have certainly had a few crashes.

Quote:

if you could point me in the right direction as to how to debug a running background process in gdb


Two methods can be used. The first is when the MUD starts it says something like:


:: Mud running in background, process id = 18439.


Now just type in:


gdb dawn 18439


That attaches it to that process.

Alternatively, to save mucking around with the background process in this case, start it like this:


gdb dawn
run -nb


The -nb flags tells DoT to not run in the background. Then you can debug normally, set breakpoints etc.

I see from the compile I get this warning:


g++ -c -Wall -g -O  -g3 who.cpp -o obj/who.o
who.cpp: In function ‘void do_who(char_data*, char*)’:
who.cpp:594: warning: operation on ‘wch’ may be undefined


Operating on undefined variables is usually bad, especially if they are pointers.

That line in the code is:


for ( wch = first_on_who; wch; wch = wch = levelsort?wch->next_who_list:wch->next_player )


The expression: wch = wch

... seems to me to not do anything useful, this may be the undefined variable part.


I am pretty sure I have used this version of Dawn before without problems. Probably the C compiler has been tightened up, and obscure coding techniques are now causing problems.

With my guidance for running gdb on the operating code, perhaps you can narrow down the problem. I had mine crash on do_who, not do_score.
- Nick Gammon

www.gammon.com.au, www.mushclient.com
[Go to top] top
Posted by Parsival   (6 posts)  [Biography] bio
Date Wed 04 Feb 2009 12:28 PM (UTC)  [ quote  ]

Amended on Wed 04 Feb 2009 01:24 PM (UTC) by Parsival

Message
I sure can:

dawn1.69r-src.tgz and dawn1.69r-support.tgz, located here:

http://www.dawnoftime.org/download/

I had to do two things to get it to compile:
Add -Wno-write-strings to C_FLAGS in ~/dot/src/configure/makefile.in, and I had to remove "lockers_object::" from line 81 of lockers.h.

This is the thread where I found that information:
http://forums.dawnoftime.org/viewtopic.php?t=2228

Also, I added -g3 to C_FLAGS because I read somewhere (perhaps in your article on the subject?) that this makes gdb more functional when debugging.

Finally, I ran ~/dot/src/configure/configure to allow the script to discover my setup and build a makefile, then in /src I ran a make.

It compiled fine (aside from some warnings), and I was able to boot the server. I created a player, logged him off, moved his file to ~/dot/player/immortal/, changed his level manually to 100, and relogged him. I saved the character, then typed 'score' and the mud crashed.

With any mortal character, the same thing happens. I tried to run the program from gdb, but because it forces itself into the background, the breakpoint I set at do_score() didn't fire.

Thanks for looking at this, Nick, and thanks for being so responsive to those of us who are less experienced. It's a rare thing, these days, to see someone help others just for the sake of helping others, and not for personal gain. Your kindness is noted and very much appreciated!

-Steve-o

Edit: Oh, if you could point me in the right direction as to how to debug a running background process in gdb, I'd love to know more about that, too!
[Go to top] top
Posted by Nick Gammon   Australia  (18,770 posts)  [Biography] bio   Forum Administrator
Date Wed 04 Feb 2009 03:44 AM (UTC)  [ quote  ]
Message
I haven't seen much activity for quite a while. The last I heard was that the developer had gone to work overseas.

Can you tell me specifically which version of DoT you downloaded (ie. file name)? I can try to reproduce the crash.
- Nick Gammon

www.gammon.com.au, www.mushclient.com
[Go to top] top

The dates and times for posts above are shown in Universal Co-ordinated Time (UTC).

To show them in your local time you can join the forum, and then set the 'time correction' field in your profile to the number of hours difference between your location and UTC time.

5,433 views.

This is page 1, subject is 2 pages long: 1 2  [Next page]

[Reply to this subject]  Reply to this subject   [New subject]  Start a new subject   [Refresh] Refresh page

Go to topic:           Search the forum

[Go to top] top

[Home]

Written by Nick Gammon - 5K

Comments to: Gammon Software support
[RH click to get RSS URL] Forum RSS feed ( http://www.gammon.com.au/rss/forum.xml )

[Best viewed with any browser - 2K]    [Internet Contents Rating Association (ICRA) - 2K]    [Web site powered by FutureQuest.Net]