Silly asteroids...

Posted by Nick Cash on Sun 28 Mar 2004 07:05 AM — 49 posts, 139,417 views.

USA #0
Ok, kind of weird, look at this:

Sat Mar 27 22:55:51 2004 :: Loading asteroids...
Sat Mar 27 22:55:51 2004 :: [*****] BUG: fread_word: EOF encountered on read.

Sat Mar 27 22:55:51 2004 :: [*****] BUG: Cannot load asteroid file: ÿ


I know for a fact there is no asteroid with the name ÿ. So, any clues to why it might be giving me this error every time it starts?

Note: Using heavily modified SWR 1.0 and this asteroid code is all custom.
USA #1
It's most likely caused by a problem with the variable type itself, or with the method by which you're reading the file. Could you post the file I/O code for it?
USA #2
Ok, this is the one. Of course like traditional file reading there are several functions involved, but this is the one in this instance me thinks.

/*
 * Load in all the asteroid files.
 */
void load_asteroids( )
{
    FILE *fpList;
    char *filename;
    char astlist[256];
    char buf[MAX_STRING_LENGTH];


    first_asteroid      = NULL;
    last_asteroid       = NULL;

    sprintf( astlist, "%s%s", AST_DIR, AST_LIST );
    fclose( fpReserve );
    if ( ( fpList = fopen( astlist, "r" ) ) == NULL )
    {
        perror( astlist );
        exit( 1 );
    }

    for ( ; ; )
    {

        filename = feof( fpList ) ? "$" : fread_word( fpList );

        if ( filename[0] == '$' )
          break;

        if ( !load_asteroid_file( filename ) )
        {
          sprintf( buf, "Cannot load asteroid file: %s", filename );
          bug( buf, 0 );
        }

    }
    fclose( fpList );
    log_string(" Done asteroids" );
    fpReserve = fopen( NULL_FILE, "r" );
    return;
}
Amended on Mon 29 Mar 2004 10:51 PM by Nick Cash
Canada #3
You might double check the asteroid list file to make sure that everything in it is right, that you have a $ at the end of the file, for example.
USA #4
Also, as stupid as this may sound - open the file in pico on a *nix machine. Go to the end. HIt backspace until you're right next to the last character in the file. Then hit enter once and resave it. Often that alone is all it takes to stop the funky symbols from showing up in the logs.
USA #5
Ok, found out something more. If I delete all of the asteroids and start a new everything goes correctly with no asteroids. If I make an asteroid and reboot I get the same error. I'm thinking it must be something with saving the asteroid. Also, if I use do_showasteroid on it, it hangs up on the line where its either showing the starsystem's name or its home starsystem (a char variable). This is set to Terra as default (STRALLOC("Terra")). Perhaps I've got the STRALLOC/str_dup thing wrong again. Would you all mind clearing that up again? DISPOSE goes with str_dup? Or what.

Thanks.
USA #6
STRALLOC goes with STRFREE and is used for hashed, or shared strings.

str_dup goes with DISPOSe and is used for nonhashed, or unique strings.

This should have no bearing on why your file is adding extra junk to the end of it. Could you post the function which saves the asteroid file?
USA #7
I just wanted to have it cleared up again, thats all. Here is save_asteroid:

/*
 * Save asteroid to a data file
 */
void save_asteroid( ASTEROID_DATA *asteroid )
{
    FILE *fp;
    char filename[256];
    char buf[MAX_STRING_LENGTH];

    if ( !asteroid )
    {
	bug( "save_asteroid: Null asteroid pointer!", 0 );
	return;
    }
        
    if ( !asteroid->filename || asteroid->filename[0] == '\0' )
    {
	sprintf( buf, "save_asteroid: %s has no filename", asteroid->name );
	bug( buf, 0 );
	return;
    }
 
    sprintf( filename, "%s%s", AST_DIR, asteroid->filename );
    
    fclose( fpReserve );
    if ( ( fp = fopen( filename, "w" ) ) == NULL )
    {
    	bug( "save_asteroid: fopen", 0 );
    	perror( filename );
    }
    else
    {
	fprintf( fp, "#ASTEROID\n" );
	fprintf( fp, "Name         %s~\n",	asteroid->name			);
	fprintf( fp, "Filename     %s~\n",	asteroid->filename		);
	fprintf( fp, "Home         %s~\n",  	asteroid->home_system   	);
	fprintf( fp, "Type         %d\n",	asteroid->type			);
	fprintf( fp, "Stype        %d\n",	asteroid->stype			);
	fprintf( fp, "Hp           %d\n",	asteroid->hp			);
	fprintf( fp, "Maxhp        %d\n",	asteroid->maxhp			);
	fprintf( fp, "Speed        %d\n",	asteroid->speed			);
	fprintf( fp, "Timer	   %d\n",	asteroid->timer			);
	fprintf( fp, "End\n\n"			);
	fprintf( fp, "#END\n"			);
    }
    fclose( fp );
    fpReserve = fopen( NULL_FILE, "r" );
    return;
}
Amended on Tue 30 Mar 2004 11:40 PM by Nick Cash
Australia Forum Administrator #8
What data is actually written to the asteroid file? Complete junk? Or valid data with a ÿ at the end? BTW I think ÿ is hex FF, that sounds suspicious.

Maybe you are passing an asteriod that is not initialised properly, or the pointer is just junk.
USA #9
Actual information is being written into it. I'll go check my asteroid creation functions again and see whats up.
USA #10
Ok, well, I've decided that I was going to put this into a snippet anyways, so here is the code (not the snippet).
http://ew.xidus.net/code/3.0a.c
http://ew.xidus.net/code/asteroids.h

Go ahead and take a look. If you want this added to your mud then feel free to use that or wait till I release a more tested version. I *THINK* that code in the .h file is the stuff I'm using in my mud, however, I'm not 100 percent positive. Let me know what you think, especially you guru's. You can prolly find a whole shit load of stuff thats wrong with it, but I think its pretty good for one of my first projects.

Thanks.
USA #11
Actually, I see a few minor things wrong with that code already. I'll go ahead and update it tomorrow, however, that code is still what I'm using (except for a few modifications). I will finish the area variable implementation the right way also.

Lata.
USA #12
Ok, got a more up to date version up. Take a look and let me know of anything you see wrong with it.
Canada #13
Quick run through of the code, and I see a few things. You used fread_string_nohash on filename, but initialized it with STRALLOC. Not a huge things, but its there. Also, this:
sprintf (aName, fread_string(fp));
Is a small memory leak.

Also, double check what the asteroid list looks like, as opposed to the individual asteroid files. Looks like you might have a bad asteroid linked into the list.
Amended on Mon 05 Apr 2004 07:09 AM by Greven
USA #14
Ok, I think I got all that fixed. Not sure if aName = fread_string(fp); is a suitable memory leak fix, but I'm sure you'll let me know. Anyways, I have since cleared the asteroid. I'll go make some asteroids and let you all know how it turns out.
Canada #15
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 ;)
USA #16
Anything else? Also, would aName = fread_string( fp ); then STRALLOC( aName ); be a suitable fix for the situation you described?
Canada #17
Well, yes, that will work, as long as you free aName afterwards.
USA #18
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.
Australia Forum Administrator #19
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.
USA #20
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?
Australia Forum Administrator #21
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.
Australia Forum Administrator #22
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.
USA #23
K, thanks. Fixed now.
USA #24
Anything else you all can find?
Australia Forum Administrator #25
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?
USA #26
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.
USA #27
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.
Amended on Fri 09 Apr 2004 02:52 AM by Nick Cash
USA #28
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.
Australia Forum Administrator #29
Sounds like the file name itself is wrong. Look at how you get the next filename from the list of names.
USA #30
Where exactly are you referring to? What function?

It appears it is having trouble in load_asteroids where it is loading the second asteroid using fread_word. Perhaps its a problem given when writing the asteroid list? If I remove all the asteroids then it goes away. However, if there is one or more asteroids then the above situation is the result, with the fread_word error and that messed up y with the two little dots.
Amended on Tue 13 Apr 2004 03:43 AM by Nick Cash
Australia Forum Administrator #31
I am worried about this error message:

BUG: Cannot load asteroid file: ÿ

This seems to have the wrong file name in it. Why I cannot say. Perhaps use gdb to step through the load asteroids function to see what is going wrong.
USA #32
I think I'm going to go through it with gdb (not really sure how, guess I'll have to go read that tutorial again). If I can't find why then I'll rewrite the functions and see what turns up. I'll keep ya posted. However, if you find anything else please tell me so.
USA #33
Ok, well, I've been stepping through load_asteroids and I'm not sure exactly what I'm looking for. It appears that the filename variable gets messed up when the for loop loops a second time, thus making it ÿ. However, before the loop has gone through once it is this:

$9 = 0xbffffa98 "Èúÿ¿Ì\035\v\b"

Got this another time:
$1 = 0xbfffeb18 "Hëÿ¿Ì\035\v\b"

I see the ÿ in there, but I don't think thats the point. I'm go gdb expert, so I really have no clue what all of this information means, I just know that the second time fread_word is called it doesn't read the next word correctly. Perhaps it isn't reading the next line?

The first thime the loop is called it reads the word using fread_word it does it correctly:

$11 = 0x82178c0 "terra1.ast"

However, when we go through the second time:

$12 = 0x82178c0 "ÿ"
Amended on Thu 15 Apr 2004 01:05 AM by Nick Cash
USA #34
I think I found something. I was stepping through the load_shuttles code and found out that just a tiny bit later on in that loop, before filename gets redefined it reads "END", which is at the end of the file of course. Anyways, in my instance, it read End instead of END. Perhaps the problem is in fact in fread_asteroid.
USA #35
Hmm, perhaps not.

Breakpoint 1, load_asteroids () at 3.0a.c:747
747             filename = feof( fpList ) ? "$" : fread_word( fpList );
(gdb) p filename
$4 = 0x81d8e85 ""
(gdb) next
749             if ( filename[0] == '$' )
(gdb) p filename
$5 = 0x82178c0 "terra1.ast"
(gdb) next
752             log_string( filename );
(gdb) next
Wed Apr 14 17:43:24 2004 :: terra1.ast
754             if ( !load_asteroid_file( (char *)filename ) )
(gdb) p filename
$6 = 0x82178c0 "terra1.ast"
(gdb) next

Breakpoint 1, load_asteroids () at 3.0a.c:747
747             filename = feof( fpList ) ? "$" : fread_word( fpList );
(gdb) p filename
$7 = 0x82178c0 "End"
(gdb) p strcpy ( filename, "END" )
$8 = 136411328
(gdb) p filename
$9 = 0x82178c0 "END"
(gdb) next
Wed Apr 14 17:43:24 2004 :: [*****] BUG: fread_word: EOF encountered on read.

749             if ( filename[0] == '$' )
(gdb) p filename
$10 = 0x82178c0 "ÿ"
Amended on Thu 15 Apr 2004 01:45 AM by Nick Cash
Australia Forum Administrator #36
Can you paste the entire contents of your asteroids list file?
USA #37

terra1.ast
$


Also, the same error is generated if more than one asteroid is contained within the list.
USA #38
Should I step through EVERY function and find exactly where the variable gets messed up? Or is there some better way to go about this?
Australia Forum Administrator #39
Does this file currently contain the exact code that is causing these problems? ...

http://ew.xidus.net/code/3.0a.c

USA #40
Yes.
Australia Forum Administrator #41
I pasted in the code from that link into db.c, and the definition for the asteroid data.

I commented out a few lines which didn't seem to relate to the problem, and which were for code I didn't want to wade through, namely:

(in the structure)
//SPACE_DATA * starsystem;
//AREAD_DATA * area;



         if ( !str_cmp( word, "Area" ) )
         {
                char *aName;
                //AREA_DATA *pArea;

                aName = fread_string(fp);

                // bad way to check how valid the area is...need to change later
//                for( pArea = first_area ; pArea ; pArea = pArea->next )
//                  if (pArea->filename && !str_cmp(pArea->filename , aName ) )
//                     asteroid->area = pArea;

                STRFREE( aName );

                fMatch = TRUE;
            }


and


//              if ( (!asteroid->hp || asteroid->hp < 1) && (!asteroid->maxhp || asteroid->maxhp < 1 ) )
//                      asteroid->hp = generate_hp( asteroid->type, asteroid->stype );
                if ( !asteroid->speed || asteroid->speed < 1 )
                        asteroid->speed = 100;
//              if ( !asteroid->starsystem )
//                      asteroid->starsystem = NULL;
//              if ( !asteroid->area )
//                      asteroid->area = NULL;



and


//              if ( asteroid->exp < 0 || !asteroid->exp )
//                      reset_asteroid( asteroid->name, asteroid->type, asteroid->stype, -1, TRUE );
 


and


//          extract_asteroid( asteroid );
//          asteroid_to_system( asteroid , starsystem_from_name(asteroid->home_system) );


I then set up an asteroids directory, and put into it your asteroid.lst file, and your terr1.ast file, make a copy of that asteroid and put the copy also into the asteroid.lst file (so I would have 2) and ran it. It seemed to work fine ...


Sat Apr 17 08:46:45 2004 :: Loading asteroids
Sat Apr 17 08:46:45 2004 :: Loading asteroids...
Sat Apr 17 08:46:45 2004 ::  Done asteroids


So, unless the commented-out code does something strange to memory (and you could test that by commenting your copy out too) I cannot explain it. :)
Amended on Fri 16 Apr 2004 11:54 PM by Nick Gammon
USA #42
I'll try commenting that stuff out and seeing what happens. Might be some confilict with other code in my codebase ::shrug::. I have no clue what that would be, but I guess you just never know. Would be kind of silly to have that nice piece of code and my own mud not be able to bennefit from it. We'll see what happens. Thanks.
USA #43
Did you comment out the functions that were related to those as well, such as extract_asteroid and the like?
Australia Forum Administrator #44
I probably didn't put them there. The reason I commented them out was to avoid the compile error. They had other structures that I didn't want to bother with. The main thing I was testing was the reading of the asteroid file, not what became of it once it was in memory.
USA #45

Fri Apr 16 22:14:49 2004 :: Loading asteroids
Fri Apr 16 22:14:49 2004 :: Loading asteroids...
Fri Apr 16 22:14:49 2004 :: terra1.ast
Fri Apr 16 22:14:49 2004 :: terra2.ast
Fri Apr 16 22:14:49 2004 :: terra3.ast
Fri Apr 16 22:14:49 2004 ::  Done asteroids

It appears commenting the stuff out fixed it. Hmm, odd. I think the problem must be in either A) extract_asteroid or B) asteroid_to_system.
USA #46
Hmm, I just thought of something. What exactly is the significance of using extract_asteroid in the load_asteroid_file function? The starsystem will ALWAYS be NULL making the function return immediately. Not only that but also extract_Asteroid is called within asteroid_to_system.
USA #47
Finally Fixed! Woot!

          extract_asteroid( asteroid );
          asteroid_to_system( asteroid , starsystem_from_name(asteroid->home_system) );

That was the malicious code, in load_asteroid_file. I replaced it with:

       if ( asteroid->timer == 0 )
	asteroid->timer = -1;

       asteroid->starsystem = starsystem_from_name( asteroid->home_system );

This allows the code to handle the addition of the asteroid to a system. Anyways, fixed version is at http://ew.xidus.net/code/3.0a.c if you wish to have a look. If not, you can just wait till the snippet version comes out. :)

Thanks to everyone, especailly Nick who got me lookin in the right spot.
Amended on Sat 17 Apr 2004 06:55 AM by Nick Cash
USA #48
Ah, finally, the glorious day. I have officialy released version 1.0 of the Asteroid snippet. You can download it from my web site by going to http://ew.xidus.net/modules.php?name=Downloads&d_op=viewdownload&cid=1 and finding the one called Code Driven asteroids. Thanks to everyone who helpped!