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 ➜ SMAUG ➜ SMAUG coding ➜ Silly asteroids...
Silly asteroids...

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

Pages: 1  2 3  4  

Posted by Greven   Canada  (835 posts)  Bio
Date Reply #15 on Mon 05 Apr 2004 07:11 AM (UTC)
Message
The memory leak comes from the call to the fread_string(fp). fread_string returns a pointer to STRALLOCed memory. By doing aName = fread_string, thats better, just make sure that you use STRFREE on it afterwards ;)
Nobody ever expects the spanish inquisition!

darkwarriors.net:4848
http://darkwarriors.net
Top
Posted by Nick Cash   USA  (626 posts)  Bio
Date Reply #16 on Tue 06 Apr 2004 05:02 AM (UTC)
Message
Anything else? Also, would aName = fread_string( fp ); then STRALLOC( aName ); be a suitable fix for the situation you described?
~Nick Cash
http://www.nick-cash.com
Top
Posted by Greven   Canada  (835 posts)  Bio
Date Reply #17 on Tue 06 Apr 2004 06:00 AM (UTC)
Message
Well, yes, that will work, as long as you free aName afterwards.
Nobody ever expects the spanish inquisition!

darkwarriors.net:4848
http://darkwarriors.net
Top
Posted by Samson   USA  (683 posts)  Bio
Date Reply #18 on Tue 06 Apr 2004 01:02 PM (UTC)
Message
One other thing I'm noticing in the fragments you've posted - when you're doing an fclose() call you aren't NULL'ing the file pointer afterwards. You should always do this because you never know when the OS will decide leaving such a thing dangling is bad. I remeber we all went through this some time back in the day when switching up from Redhat 6 to Redhat 7.
Top
Posted by Nick Gammon   Australia  (23,173 posts)  Bio   Forum Administrator
Date Reply #19 on Wed 07 Apr 2004 02:12 AM (UTC)
Message
Sampson, the "fp" you are talking about goes out of scope anyway in a couple of lines, in that function. I don't see how the operating system can complain if you don't NULL a pointer like that.
- Nick Gammon

www.gammon.com.au, www.mushclient.com
Top
Posted by Nick Cash   USA  (626 posts)  Bio
Date Reply #20 on Wed 07 Apr 2004 03:42 AM (UTC)
Message
Actually, the aName variable is only needed to search through the areas in finding the correct one. After that, it has no purpose. So, knowing that, does it really need to be allocated?
~Nick Cash
http://www.nick-cash.com
Top
Posted by Nick Gammon   Australia  (23,173 posts)  Bio   Forum Administrator
Date Reply #21 on Wed 07 Apr 2004 04:37 AM (UTC)
Message
Quote:

Also, would aName = fread_string( fp ); then STRALLOC( aName ); be a suitable fix for the situation you described?


This is allocating it twice. You mean STRFREE (aName);

Quote:

So, knowing that, does it really need to be allocated?


The fread_string routine allocates inside itself, like this:


        case '~':
            *plast = '\0';
            return STRALLOC( buf );


Thus, the string is already allocated, and must be freed if you don't want the memory leak. It doesn't matter what you plan to do with it, if anything.
- Nick Gammon

www.gammon.com.au, www.mushclient.com
Top
Posted by Nick Gammon   Australia  (23,173 posts)  Bio   Forum Administrator
Date Reply #22 on Wed 07 Apr 2004 04:40 AM (UTC)
Message
There is a difference, BTW, in fread_word, which does not allocate memory. However that leads to a different trap, if you are not careful.

eg.


char * word1 = fread_word (fp);
char * word2 = fread_word (fp);

if (strcmp (word1, "foo") == 0) 
  {
  // do something
  }


Now this code will not have a memory leak, because fread_word does not allocate memory. However it has a different problem. fread_word does not allocate memory because it returns a pointer to a *static* buffer. Thus, word1 and word2 in the example above are the same piece of memory, and word1 is no longer what was read, because word2 has now replaced it.
- Nick Gammon

www.gammon.com.au, www.mushclient.com
Top
Posted by Nick Cash   USA  (626 posts)  Bio
Date Reply #23 on Wed 07 Apr 2004 05:17 AM (UTC)
Message
K, thanks. Fixed now.
~Nick Cash
http://www.nick-cash.com
Top
Posted by Nick Cash   USA  (626 posts)  Bio
Date Reply #24 on Thu 08 Apr 2004 02:51 AM (UTC)
Message
Anything else you all can find?
~Nick Cash
http://www.nick-cash.com
Top
Posted by Nick Gammon   Australia  (23,173 posts)  Bio   Forum Administrator
Date Reply #25 on Thu 08 Apr 2004 10:54 PM (UTC)
Message
Quote:

Actual information is being written into it.


In this case it still isn't clear if the problem is the writing or the reading.

Can you paste what is being written, and the code that reads it back, and describe exactly what goes wrong?
- Nick Gammon

www.gammon.com.au, www.mushclient.com
Top
Posted by Nick Cash   USA  (626 posts)  Bio
Date Reply #26 on Fri 09 Apr 2004 01:31 AM (UTC)
Message
Ok, I got the showasteroid thing ironed out. I'll format it later. Anyways, I created a new asteroid with the filename terra1.ast. This is what was in the file.

#ASTEROID
Name         Rigel~
Filename     terra1.ast~
Home         Terra~
Type         1
Stype        1
Hp           4000
Maxhp        4000
Speed        100
Timer        0
End

#END

Thats after I set some stuff and use resetasteroid on it. Next, I shutdown the mud. I rebooted, and in the startup log I get:

Thu Apr  8 17:28:35 2004 :: [*****] BUG: fread_word: EOF encountered on read.

Thu Apr  8 17:28:35 2004 :: [*****] BUG: Cannot load asteroid file: ÿ

The asteroid.lst file reads (just for reference):

terra1.ast
$

The current code I'm running (with today's revision's of the do_showasteroid function) can be found at http://ew.xidus.net/code/3.0a.c

In there, the file reading/saving functions are fread_asteroid and save_asteroid. So take a look and let me know if you find anything.

Thanks.
~Nick Cash
http://www.nick-cash.com
Top
Posted by Nick Cash   USA  (626 posts)  Bio
Date Reply #27 on Fri 09 Apr 2004 02:51 AM (UTC)

Amended on Fri 09 Apr 2004 02:52 AM (UTC) by Nick Cash

Message
After some extensive testing I've found that it doesn't load any asteroid but the first. I'm gunna go investigate a bit more.

BTW, it loads it correctly.
~Nick Cash
http://www.nick-cash.com
Top
Posted by Nick Cash   USA  (626 posts)  Bio
Date Reply #28 on Mon 12 Apr 2004 10:20 PM (UTC)
Message
So, any more thoughts? Anything else in the code that looks vile and needs to be banished from the realm of coding?

Thanks a bunch.
~Nick Cash
http://www.nick-cash.com
Top
Posted by Nick Gammon   Australia  (23,173 posts)  Bio   Forum Administrator
Date Reply #29 on Tue 13 Apr 2004 01:19 AM (UTC)
Message
Sounds like the file name itself is wrong. Look at how you get the next filename from the list of names.
- Nick Gammon

www.gammon.com.au, www.mushclient.com
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.

135,214 views.

This is page 2, subject is 4 pages long:  [Previous page]  1  2 3  4  [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.

Home