Minulla on sellainen pieni ongelma, että rivi
if ( !fgets(temp, sizeof(temp), pFile) ) { //Read a line of text
printf("\nSome error with fgets.\n");
}ilmeisesti palauttaa arvoa 0 jonkin aikaa sen jälkeen, kun FILE *pFile on vaihdettu osoittamasta config.txt:stä scripts/menus.txt:hen. Se siis onnistuneesti looppaa tuota readscript()-funktiota monta kertaa ja sitten lakkaa toimimasta. Linuxissa kaatuu segmentation faulttiin siinä vaiheessa kuitenkin jonkin aikaa vielä lukien sitä menus.txt:tä samalla päästellen "Some error with fgets."-debuggausviestiä kunnes päävalikossa painetaan return-näppäintä painikkeen "Start a new game" kohdalla, jatkaen edelleen riville riville 50 menus.txt:ssä, jonka sisältö on seuraavanlainen:
step 4 text "Dwarves: Dwarves are sometimes allied with humans but many of them fear goblins so"
ja kaatuen segmentation faulttiin, mutta Windowsissa jatkaa ohjelman suoritusta silti tosin tulostellen "Some error with fgets.".
gdb sanoo:
Program received signal SIGSEGV, Segmentation fault.
_IO_fgets (buf=0x805ad80 "\tstep 4 text \"Dwarves: Dwarves are sometimes allied with humans but many of them fear goblins so\"\r\n", n=180, fp=0x8006f73)
at iofgets.c:52
52 _IO_acquire_lock (fp);
(gdb) bt
#0 _IO_fgets (buf=0x805ad80 "\tstep 4 text \"Dwarves: Dwarves are sometimes allied with humans but many of them fear goblins so\"\r\n", n=180,
fp=0x8006f73) at iofgets.c:52
#1 0x0804be62 in readscript () at src/tom.c:788
#2 0x0804d1fb in main (argc=1, argv=0xbffff294) at src/tom.c:1050Rivin 788 sisältö tom.c:ssä:
if ( !fgets(temp, sizeof(temp), pFile) ) { //Read a line of textRivin 1050 sisältö tom.c:ssä:
while( readscript() == 0 ); //An infinite loop that reads the open script file time after time.
Ja iofgets.c on standardikirjaston osa.
Valgrindilläkin tuli katsottua onko muistivuotoja, mutta mitään kovin isoa ei ainakaan ole.
Koko koodin voi ladata tästä (päivittelen sitä tosin vähän väliä): http://mandicor.net/tom/tom.zip
Tässä kuitenkin pätkä koodista, jossa hyvin todennäköisesti virhe on:
tom.h:
char temp[180]; char whatmenu[50]; char *pointer; //Used by parser /* Tässä siis vain näitä parserin kannalta olennaisimpia muuttujia. */
tom.c:
/* Ongelman kannalta epäoleellisia funktioita poistettu */
/*
My own parser. Perhaps it would've been better to use LUA or something
like that but I didn't.
This function returns a pointer to the number that follows the string
it was called with. If unsuccessful, it will return integer value 0,
otherwize 1.
Great for checking if there is a certain character string in the line
it is reading.
*/
int getpointer(char string[]) {
int i;
int j = 0;
int x = 1;
int whatlen = strlen(string);
int wherelen = strlen(temp);
for( i = 0; i <= wherelen - whatlen; ++i) {
for(j = 0; j < whatlen; ++j) {
if(string[j] != temp[i+j]) break;
}
if(j == whatlen) break;
}
if(j != whatlen) return 0;
for (j=0; j<=i; j++) {
if (temp[j]=='\"') ++x;
if ( j == i && !( x % 2 ) ) return 0; //Return if preceded by an uneven number of double quotes
}
pointer = temp + i;
pointer += whatlen;
if (*pointer + 1 > 'a' && *pointer + 1 < 'z') return 0;
while (*pointer == '\"' || *pointer == '=' || *pointer == ' ') pointer++;
return 1;
}
void readstring( char to[] ) { //This function copies the contents of the pointer to temp to the character string it was called with.
int i = 0;
do {
to[i]=*pointer;
pointer++;
i++;
} while (*pointer != '\"');
to[i] = '\0';
}
/* Compares the pointer and a string. strcmp probably could do it too. */
int comparestrings( char to[] ) {
int i = 0;
int j = strlen(to);
do {
pointer++;
i++;
if ( i >= j ) return 1;
} while (*pointer == to[i]);
return 0;
}
/*
This function is a modification of stcmp function. It tests if the strings are the same with no additional characters following.
*/
int cstr(char from[], char to[]) {
int i = strlen(to);
if (!strcmp(from, to) ) { //compare strings
if ( to[i] > 'a' && to[i] < 'z' ) return 0;
if ( from[i] < 'a' || from[i] > 'z' ) return 1; //if no more characters present return 1
}
return 0;
}
/*
The Parser.
*/
int readscript() {
SDL_EventState(SDL_KEYUP, SDL_IGNORE); //We don't support keyups at the moment.
static char readfunction[50] = "config.txt"; //Which functions do we wish to read?
FILE *pFile;
int i, j, k, a, finalcolor, truthvalue, formeri, latteri, textpx, textpy, shallwehighlight = -1, wastext, braces;
float menuitems; //Used in menu layout
char tmpvar[80];
char calc[4][30];
#ifdef withmusic
static char whichmusic[50] = { '\0' };
#endif
pFile = fopen (readfunction,"r");
if (!pFile) {
printf("Error opening file at %s.\n", readfunction);
perror("");
}
while( 1 ){ //Because of how feof works
if ( !fgets(temp, sizeof(temp), pFile) ) { //Read a line of text
printf("\nSome error with fgets.\n");
}
if ( feof(pFile) ) break;
if (getpointer("if")) { //At the moment conflicts with assignment statements so it should be the first one here.
do {
truthvalue = 1;
do {
readstring(calc[0]); //The former operand
pointer++; //Double quotes.
readstring(calc[2]); //The operation mark
pointer++;
readstring(calc[1]); //The latter operand
pointer++;
readstring(calc[3]); //The && and ||
pointer++;
formeri = latteri = 0;
for (i = 0; i < vartabvalues; i++) {
if ( cstr(calc[0], varTable[i].name ) )
formeri = i;
if ( cstr(calc[1], varTable[i].name ) )
latteri = i;
}
if (latteri==0) latteri=1;
/*
This part is for constant integers.
*/
for (a = 0; a < 2; a++) {
i = 0;
if ((a == 0 && formeri == 0) || (a == 1 && latteri == 1)) {
k = 0;
while ((calc[a][i] < '0' && calc[a][i] != '\0') || (calc[a][i] > '9' && calc[a][i] != '\0')) {
if (calc[a][i] == 'k') {k = 1; break;}
if (calc[a][i] == 'K') {k = 2; break;}
tmpvar[i] = calc[a][i]; //We still need the operators
calc[a][i]=' ';
i++;
}
if ((k == 0)) *varTable[a].var.d = atoi(calc[a]);
if ((k == 1)) *varTable[a].var.d = calc[a][i+1];
if ((k == 2)) *varTable[a].var.d = calc[a][i+1] + 208; //Keypad numbers
for ( j = 0; j<i; j++) calc[a][j] = tmpvar[j]; //Put them back
}
}
if( strstr( calc[2], "=" ) ) {
if ( *varTable[formeri].var.d != *varTable[latteri].var.d ) { truthvalue = 0; break; }
}
} while ( strstr(calc[3], "&&") );
if (truthvalue == 1) {
printf("\nStatement is TRUE!\n");
if ( !(fgets(temp, sizeof(temp), pFile) ) ) { //Because of the { ... } that would otherwise be skipped
printf("Error in an if fgets");
exit(0);
}
break; //Statement is still true after all &&s
}
} while ( strstr(calc[3], "||") );
}
if (!getpointer("if") && !getpointer("int") && !getpointer("char")) { //They are in conflict with assignment statements.
for (i = 0; i < vartabvalues; i++) { //Read each variable defined in varTable
if (getpointer(varTable[i].name)) {
if ( varTable[i].type == 0 ) {
*varTable[i].var.d = atoi(pointer);
if (debug == 1) { printf("%s%d ", varTable[i].name, *varTable[i].var.d); }
}
if ( varTable[i].type == 1 ) {
readstring(varTable[i].var.s);
if ( cstr(varTable[i].name, "whatmenu" )) menuitem = 0;
if (debug == 1) { printf(" %s", varTable[i].var.s); }
}
if ( varTable[i].type == 2 ) {
*varTable[i].var.e = atof(pointer);
if (debug == 1) { printf("%s%d ", varTable[i].name, *varTable[i].var.d); }
}
if ( varTable[i].type == 3 ) {
varTable[i].var.i = atoi(pointer);
if (debug == 1) { printf("%s ", varTable[i].name); }
}
if ( varTable[i].type == 4 ) {
readstring(varTable[i].var.c);
if (debug == 1) { printf(" %s", varTable[i].var.c); }
}
}
}
}
if (getpointer("initializegame")) initialize();
if ( getpointer("int") ) {
if ( vartabvalues >= sizeof(varTable) ) { printf("TODO: Memory allocation for new variables. Exiting..."); exit(0); }
readstring( tmpvar );
for (i = 0; i < vartabvalues; i++) { //Read each variable defined in varTable
if ( cstr(varTable[i].name, tmpvar) ) break;
}
if ( i == vartabvalues ) { //This prevents duplicates.
strcpy (varTable[vartabvalues].name, tmpvar);
varTable[vartabvalues].var.i = atoi(pointer + 1);
varTable[vartabvalues].type = 3;
printf("A new variable defined: %s %d\n", varTable[vartabvalues].name, varTable[vartabvalues].var.i);
vartabvalues++;
}
};
if ( getpointer("char") ) {
if ( vartabvalues >= sizeof(varTable) ) { printf("TODO: Memory allocation for new variables. Exiting..."); exit(0); }
readstring( tmpvar );
for (i = 0; i < vartabvalues; i++) { //Read each variable defined in varTable
if ( cstr(tmpvar, varTable[i].name) ) break;
}
if ( i == vartabvalues ) {
strcpy (varTable[vartabvalues].name, tmpvar);
while (*pointer == '\"' || *pointer == '=' || *pointer == ' ') pointer++;
readstring( varTable[vartabvalues].var.c );
varTable[vartabvalues].type = 4;
printf("A new character array defined: %s %s\n", varTable[vartabvalues].name, varTable[vartabvalues].var.c);
vartabvalues++;
}
};
if (getpointer("run")) readstring(readfunction);
if (getpointer("exit")) return 1; //exit(1);
if (getpointer("caption")) { readstring(tmpvar); SDL_WM_SetCaption(tmpvar, NULL); } //The name of the window
if (getpointer("{")) {
if (getpointer("createmenu") && comparestrings(whatmenu) ) {
divisor = 0, size = 1, color = 1, centered = 0, xcoords = 0, xdivis = 0, step = 1, menuitems = 0, wastext = 0;
SDL_FillRect( SDL_GetVideoSurface(), NULL, 0 ); //This will clear the screen
}
else {
braces = 1;
while ( braces ) {
if ( !(fgets(temp, sizeof(temp), pFile) ) ) { printf("Error in a function fgets"); exit(0); }
if ( getpointer("{") ) braces++;
if ( getpointer("}") ) braces--;
}
}
}
if (getpointer("finishtext")) {
SDL_Flip(scrn);
if ( menuitem > shallwehighlight ) menuitem = 0;
if ( menuitem < 0 ) menuitem = shallwehighlight;
shallwehighlight = 0;
}
if (getpointer("text")) {
textpx = textpy = 0;
menuitems += step;
if ( color < 0 ) {
if (wastext == 0) shallwehighlight++;
wastext = 1;
if (shallwehighlight == menuitem) finalcolor = 1;
else finalcolor = 0;
}
else finalcolor = color;
readstring(tmpvar);
if (xdivis<=0) textpx = xcoords * fsz;
else textpx = RES_X / xdivis + xcoords * fsz;
if ( divisor > 0 ) textpy = RES_Y / divisor * menuitems;
if (centered) centeredtext(finalcolor, tmpvar, textpy, fsz * size);
else fixedtext(finalcolor, tmpvar, textpx, textpy, fsz * size);
}
else wastext = 0;
if (getpointer("waitpressedkey")) {
status = SDL_WaitEvent(&event); //wait indefinitely for an event to occur
if ( !status ) { //Error has occurred while waiting
printf("SDL_WaitEvent error: %s\n", SDL_GetError());
return 0;
}
if ( event.type == SDL_KEYDOWN ) keyp = event.key.keysym.sym; //Copy the just pressed key's value to keyp
if ( event.type == SDL_QUIT ) {
SDL_FillRect( SDL_GetVideoSurface(), NULL, 0 );
centeredtext(1, "Really quit? y/n", RES_Y / 2 - fsz*1.5, fsz*3);
if (wait_y()) exit(1);
pois=1;
}
if ( keyp == SDLK_ESCAPE ) {
SDL_FillRect( SDL_GetVideoSurface(), NULL, 0 );
centeredtext(1, "Really quit? y/n", RES_Y / 2 - fsz*1.5, fsz*3);
if (wait_y()) exit(1);
pois=1;
}
if ( keyp == SDLK_h ) { ingamemenu(); SDL_FillRect( SDL_GetVideoSurface(), NULL, 0 ); SDL_Delay(delay); } //List of commands
if ( keyp == SDLK_0 ) settings(); //Settings
if ( keyp == SDLK_RETURN ) {
if ( cstr(whatmenu, "main") ) {
if ((menuitem == 2)) { ingamemenu(); }
if ((menuitem == 4)) exit(1);
}
}
if (event.type == SDL_VIDEORESIZE) {
leveys=event.resize.w+leveys-RES_X;
korkeus=event.resize.h+korkeus-RES_Y;
RES_X = event.resize.w;
RES_Y = event.resize.h;
resize();
}
}
if (getpointer("verticalarrows")) {
if ( keyp == SDLK_UP ) menuitem--;
if ( keyp == SDLK_KP8 ) menuitem--;
if ( keyp == SDLK_DOWN ) menuitem++;
if ( keyp == SDLK_KP2 ) menuitem++;
}
if (getpointer("horizontalarrows")) {
if ( keyp == SDLK_LEFT ) menuitem--;
if ( keyp == SDLK_KP4 ) menuitem--;
if ( keyp == SDLK_RIGHT ) menuitem++;
if ( keyp == SDLK_KP6 ) menuitem++;
}
#ifdef withmusic
if (playsound!=0) {
if (getpointer("changemusic")) {
readstring(tmpvar);
if ( strcmp(whichmusic, tmpvar ) ) {
strncpy (whichmusic, tmpvar, strlen(tmpvar));
if (debug == 1) printf("\nChanged music.\n");
sound = Mix_LoadMUS(tmpvar);
if(sound == NULL) {
printf("Unable to load music file %s: %s\n", tmpvar, Mix_GetError());
playsound=0;
}
if(musicPlaying<1) {
if(Mix_PlayMusic(sound, 0) == -1)
{
printf("Unable to play music file: %s\n", Mix_GetError());
}
}
musicPlaying = 1;
}
}
}
#endif
}
if ( fclose (pFile) != 0 ) printf("Error when closing file.\n");
return 0;
}
/*
The main loop. This is where it all begins.
*/
int main(int argc, char *argv[]){
while( readscript() == 0 ); //An infinite loop that reads the open script file time after time.
/* Poistettu command line optionit ja loppu main:sta */Ensimmäinen itse koodaamani C-ohjelma, muuten.
Kaatuminen valikkoon mennessä johtuu niinkin perinteisestä syystä kuin puskurin ylivuodosta readstring-funktiossa. Sinun pitää siis ilmoittaa readstring-funktiolle puskurin pituus ja huolehtia, ettei tekstiä tule luettua sen enempää. Itse toteutin tarkistuksen testimielessä tällaisella makrolla:
#define readstring2(x) readstring_(x, (sizeof(x) == sizeof(char*)) ? 50 : sizeof(x))
(Makron toiminta perustuu oletukseen, että ainoat käyttämäsi osoittimet ovat noissa varTable[i].var.s -kohdissa ja että niihin mahtuu 50 merkkiä. Makro ei sovi lopulliseksi korjaukseksi!)
Ohjelman lopussa sattuva kaatuminen johtuu siitä, että vapautat joitain pintoja moneen kertaan. Yksinkertaisin korjaus tähän on, että asetat aina vapautuksen jälkeen osoittimeen arvon NULL. Itse toteutin tämän testimielessä koodiisi korvaamalla kaikki SDL_FreeSurface-kutsut seuraavalla makrolla:
#define SDL_Safe_FreeSurface(x) (SDL_FreeSurface(x), (void)(x = 0))
Resurssivuotoja on ainakin kuvien latauksissa. Jos avaat tiedoston funktiolla SDL_RWFromFile, se pitää vastaavasti sulkea funktiolla SDL_RWclose. Viisainta olisi käyttää kuvien lataukseen suoraan funktiota IMG_Load ilman mitään omia kikkailuja.
Jaa-ah. Ei ihan mitä gdb antoi ymmärtää. Tosiaan nuo tekstit ovat huomattavasti pidempiä kuin bufferi.
Aika harvoin sattuu virheitä, jotka gdb osaisi suoraan ilmoittaa perin pohjin; yleensä pitää osata tulkita ja tutkia tarkemmin. Puskurin ylivuoto selvisi siten, että kaatumisen jälkeen palasin up-komennolla readscript-funktioon ja tarkistin fgets:lle annetut parametrit. Muuttujan pFile arvo oli kerta kaikkiaan sopimaton osoittimelle. (Jos kokemus ja pohjatiedot eivät riitä, sopimattomuuden voi tarkistaa tulostamalla: print *pFile ilmoittaa, että muistia ei voida lukea.) Muuttujien oudot arvot taas lähes aina johtuvat mokailusta taulukoiden pituuden kanssa, ja sinun ohjelmassasi tästä ajatuksesta on aika lyhyt matka omatekoiseen readstring-funktioon.
Ohjelman lopussa Linuxilla tulee ilmoitus "double free or corruption", jonka tulkintaan ei tarvita edes gdb:tä, koska tyypillisiä syitä on vain kaksi: ilmoituksessa mainittu kaksoisvapautus (kuten sinulla) ja puskurin yli- tai alivuoto.
Pari muuta suunnitteluvirhettä jäi mainitsematta: Jos tiedostoa ei saada auki, on aika tyhmää mennä ikuiseen silmukkaan, jossa sitä yritetään lukea. Loogisempaa olisi keskeyttää funktio saman tien. Vastaavasti jos fgets menee pieleen, break olisi paljon mielekkäämpi ratkaisu kuin silmukan jatkaminen.
Aihe on jo aika vanha, joten et voi enää vastata siihen.