Register forum user name Search FAQ

Gammon Forum

Notice: Any messages purporting to come from this site telling you that your password has expired, or that you need to verify your details, confirm your email, resolve issues, making threats, or asking for money, are spam. We do not email users with any such messages. If you have lost your password you can obtain a new one by using the password reset link.

Due to spam on this forum, all posts now need moderator approval.

 Entire forum ➜ Dawn of Time ➜ Administration ➜ Two Questions: a) Code Still Supported? b) Crashing somewhere in show_score()

Two Questions: a) Code Still Supported? b) Crashing somewhere in show_score()

It is now over 60 days since the last post. This thread is closed.     Refresh page


Pages: 1 2  

Posted by Parsival   (6 posts)  Bio
Date Wed 04 Feb 2009 02:27 AM (UTC)
Message
Hi, and thanks for reading my post!

First question: As I peruse the dawnoftime.org site and try (unsuccessfully) to activate an account on the forums there, I notice that there hasn't been much activity surrounding this codebase on the forums for quite some time... Has this project died, or is someone still working it?

It's a pretty hot codebase, and I'm anxious to learn more about it!

Second question: I'm running Ubuntu 8.10, and was successful in getting the source to compile and get the server running. I've created a couple characters, one of which is now the administrator.

When I type 'score', the mud crashes immediately.

I'm still a little unfamiliar with gdb, and don't know how to load up a background process in order to set breakpoints in the code, so I am unsure where the buffer overrun is happening. If you might have a moment to look through this backtrace and give me some suggestions on how to proceed, I'd be very grateful.

Backtrace:

#0  0xb7f49430 in __kernel_vsyscall ()
#1  0xb7cae880 in raise () from /lib/tls/i686/cmov/libc.so.6
#2  0xb7cb0248 in abort () from /lib/tls/i686/cmov/libc.so.6
#3  0xb7cec10d in ?? () from /lib/tls/i686/cmov/libc.so.6
#4  0xb7d7d548 in __fortify_fail () from /lib/tls/i686/cmov/libc.so.6
#5  0xb7d7b670 in __chk_fail () from /lib/tls/i686/cmov/libc.so.6
#6  0xb7d7ad68 in ?? () from /lib/tls/i686/cmov/libc.so.6
#7  0xb7cf0a18 in _IO_default_xsputn () from /lib/tls/i686/cmov/libc.so.6
#8  0xb7cc3083 in vfprintf () from /lib/tls/i686/cmov/libc.so.6
#9  0xb7d7ae14 in __vsprintf_chk () from /lib/tls/i686/cmov/libc.so.6
#10 0xb7d7ad5d in __sprintf_chk () from /lib/tls/i686/cmov/libc.so.6
#11 0x080fda0e in get_ictimediff (t1=1196692920, t2=1233712584, 
    icyears_added_to_result=0) at /usr/include/bits/stdio2.h:35
#12 0x0819b5ac in show_score (ch=0xb74e83d0, v=0xb74e83d0) at score.cpp:126
#13 0x0819c4f8 in do_score (ch=0xb74e83d0, argument=0x83daa05 "")
    at score.cpp:284
#14 0x08100eae in interpret (ch=0xb74e83d0, argument=0x83daa05 "")
    at interp.cpp:1322
#15 0x080afc84 in process_input (c=0xb74ea1fc) at comm.cpp:3875
#16 0x08156c9b in netio_process_input_from_polled_connections ()
    at netio.cpp:1259
#17 0x080b460d in game_loop () at comm.cpp:1111
#18 0x080b495c in main (argc=1, argv=0xbf848e44) at comm.cpp:743



Top

Posted by Nick Gammon   Australia  (23,133 posts)  Bio   Forum Administrator
Date Reply #1 on Wed 04 Feb 2009 03:44 AM (UTC)
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
Top

Posted by Parsival   (6 posts)  Bio
Date Reply #2 on Wed 04 Feb 2009 12:28 PM (UTC)

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!
Top

Posted by Nick Gammon   Australia  (23,133 posts)  Bio   Forum Administrator
Date Reply #3 on Thu 05 Feb 2009 02:29 AM (UTC)
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
Top

Posted by Nick Gammon   Australia  (23,133 posts)  Bio   Forum Administrator
Date Reply #4 on Thu 05 Feb 2009 02:31 AM (UTC)
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
Top

Posted by Parsival   (6 posts)  Bio
Date Reply #5 on Thu 05 Feb 2009 11:47 PM (UTC)
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
Top

Posted by Nick Gammon   Australia  (23,133 posts)  Bio   Forum Administrator
Date Reply #6 on Fri 06 Feb 2009 02:50 AM (UTC)
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
Top

Posted by Parsival   (6 posts)  Bio
Date Reply #7 on Fri 06 Feb 2009 12:22 PM (UTC)
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
Top

Posted by Nick Gammon   Australia  (23,133 posts)  Bio   Forum Administrator
Date Reply #8 on Fri 06 Feb 2009 07:42 PM (UTC)
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
Top

Posted by ThomasWatts   USA  (66 posts)  Bio
Date Reply #9 on Sat 07 Feb 2009 08:33 AM (UTC)
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.
Top

Posted by Nick Gammon   Australia  (23,133 posts)  Bio   Forum Administrator
Date Reply #10 on Sat 07 Feb 2009 09:44 PM (UTC)
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
Top

Posted by ThomasWatts   USA  (66 posts)  Bio
Date Reply #11 on Sun 08 Feb 2009 06:26 AM (UTC)
Message
Yes, perhaps it should be wch == levelsort
Check for comparison then assign.
Top

Posted by David Haley   USA  (3,881 posts)  Bio
Date Reply #12 on Mon 09 Feb 2009 01:16 PM (UTC)

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
Top

Posted by Nick Gammon   Australia  (23,133 posts)  Bio   Forum Administrator
Date Reply #13 on Mon 09 Feb 2009 06:45 PM (UTC)
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
Top

Posted by David Haley   USA  (3,881 posts)  Bio
Date Reply #14 on Mon 09 Feb 2009 07:00 PM (UTC)

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
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.


56,003 views.

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

It is now over 60 days since the last post. This thread is closed.     Refresh page

Go to topic:           Search the forum


[Go to top] top

Information and images on this site are licensed under the Creative Commons Attribution 3.0 Australia License unless stated otherwise.