Program crashes if I don't add a system("PAUSE") before calling a method

[zyky]

Hello,
I'm writing a simple program implementing a list. You can add, remove, search, print and find the minimum element.
My problem is that the program crashes if I don't add the 'system("PAUSE");' line before the last 'printf("Minimo = %d.\n", minimo(l));' call in the main method.

Here is the code:

#include <cstdlib>
#include <iostream>
#include <limits.h>

using namespace std;

/****************************************************************************/
struct Record {
    int val;                  // the value
    struct Record* next;      // pointer to the next Record
};

/****************************************************************************/
struct Lista {
    Record* testa;            // pointer to the first element in the list
};

/****************************************************************************/
// returns the pointer to the record with value k, or NULL
void* search(struct Lista* l, int k);
// delete the record pointed by x
void cancella(struct Lista* l, Record* x);
// prints the list
void stampaLista(struct Lista* l);
// add a new element to the list
void aggiungi(struct Lista* l, int val);
// returns the value of the minimum element in the list
int minimo(struct Lista* l);
// returns true if the list is empty
bool isEmpty(struct Lista* l);

/****************************************************************************/
void cancella(struct Lista* l, Record* x) {
    if(isEmpty(l))
        return;

    // new pointer to the head of the list
    struct Record* r = (struct Record*) malloc(sizeof(struct Record));
    r = l->testa;

    // if I have to delete the head of the list
    if(l->testa == x) {
        l->testa = l->testa->next;
        free(r);
        free(&r);
        return;        
    }
    
    // else,
    do {
        // if the element to delete is the 'next' one
        if(r->next == x) {
            void* v = r->next->next;
            free(r->next);
            r->next = (Record*) v;
            break;
        }
    } while( (r = r->next) != NULL);
    free(&r);
        
}


/****************************************************************************/
// prints
void stampaLista(struct Lista* l) {

    if(isEmpty(l)) {
        printf("La Lista è vuota.\n");
        return;
    }

    struct Record* r = (struct Record*) malloc(sizeof(struct Record));
    r = l->testa;

    do {
        printf("%d, ", r->val);
    } while( (r = r->next) != NULL);
    printf("\n");

    free(&r);
}

/****************************************************************************/
bool isEmpty(Lista* l) {
    if(l->testa == NULL)
        return true;
    return false;     
}

/****************************************************************************/
void* search(struct Lista* l, int k) {
    if(isEmpty(l))
        return NULL;

    struct Record* r = (struct Record*) malloc(sizeof(struct Record));
    r = l->testa;

    do {
        if(r->val == k) {
            return r;
        }
    } while( (r = r->next) != NULL);

    free(&r);
    
    return NULL;
}

/****************************************************************************/
int minimo(struct Lista* l) {
    if(isEmpty(l))
        return INT_MAX;

    int min = INT_MAX;
        
    struct Record* r = (struct Record*) malloc(sizeof(struct Record));
    r = l->testa;

    do {
        if(r->val < min)
            min = r->val;
    } while( (r = r->next) != NULL);

    free(&r);

    return min;    
}


/****************************************************************************/
void aggiungi(struct Lista* l, int val) {
    // creates the new Record
    struct Record* r = (struct Record*) malloc(sizeof(Record));
    r->val = val;
    r->next = NULL;

    // if the list is empty, the new record is the head of the list
    if(isEmpty(l)) {
        l->testa = r;
        return;           
    }

    // else, search the last element in the list and appends the new Record
    struct Record* r2 = (struct Record*) malloc(sizeof(struct Record));
    r2 = l->testa;

    do {
        if(r2->next == NULL) {
            r2->next = r;
            break;
        }
    } while( (r2 = r2->next) != NULL);

    free(&r2);
    
     
}


/****************************************************************************/
int main(int argc, char *argv[]) {
    
    Lista* l = (Lista*) malloc(sizeof(Lista));
    l->testa = NULL;
    
    stampaLista(l);
    aggiungi(l, 10);
    aggiungi(l, 20);
    aggiungi(l, 30);
    aggiungi(l, 40);
    aggiungi(l, 50);
    stampaLista(l);
    
    cancella(l, (Record*) search(l, 30));    
    
    stampaLista(l);
    
    printf("Minimo = %d.\n", minimo(l));

    cancella(l, (Record*) search(l, 10));    
    
    stampaLista(l);

    //////////////////           HERE HERE HERE        /////////////////////
    // IF I TAKE THIS OUT, IT CRASHES
    system("PAUSE");
    printf("Minimo = %d.\n", minimo(l));
    
    system("PAUSE");
    return EXIT_SUCCESS;
}

I really have no idea :(
Thank you in advance.

[thelamb]

Are you compiling a 'debug' build?
If so, you should get an ASSERT much earlier.

Look at your aggiungi function (btw, why are you mixing english and other-language variable/function names? It can become very confusing).

In this aggiungi function, why are you creating r2 on the heap(e.g. why are you using malloc).
Let's assume you keep the malloc, then at some point you need to free it, the prototype for free is:
free( void* )

But you are handing over a reference to r2 (&r2), that will evaluate to 'struct Record **' instead of 'struct Record *'.

Then in the calcella function, you are doing:

free(r);
        free(&r);

Which can never be right.

After you fix all the free's the program still crashes though.. but that is an excersize for you ;)

[zyky]

Thank you for your suggestions thelamb.

'Unfortunately' I know nothing about debugging, so there is no debugging code or anything (sooner or later I'll have to study how it works).

I'm sorry for the mixed language, I added some english just in the post here, to make it a little more understandable.

I have to say that I'm pretty new to C++ (I'm using Java almost always), so I'm still confused about pointers and mallocs.

I took out all the malloc that seem unnecessary (to me). For example the aggiungi (add) method now looks like this:

void aggiungi(struct Lista* l, int val) {
    // the new Record
    struct Record* r = (struct Record*) malloc(sizeof(Record));
    r->val = val;
    r->next = NULL;

    // if the list is empty, the new Record becomes the head of the list
    if(isEmpty(l)) {
        l->testa = r;
        return;           
    }

    // else, searches for the last value in the list and appends the new one.
    struct Record* r2 = l->testa;
    do {
        if(r2->next == NULL) {   // if it is the last value
            r2->next = r;        // make it point to the new value
            break;            
        }
    } while( (r2 = r2->next) != NULL);
    
}

In this method the 'malloc()' is necessary, isn't it? I think that 'r' would be destroyed when the method ends if I don't use 'malloc()', but I may be wrong.

Ah, about the cancella function (delete function):

free(r);
free(&r);

I wrote this because I wanted to delete (and free) both the Record pointed by 'r' and the pointer 'r' itself. Is it wrong?

Thank you for your explanations, even though the original problem is not solved I'm learning useful stuff!

[thelamb]

Well actually, you are writing C code here.. in C++ you don't really use malloc explicitly.

So I suggest you read some basic C++ books (there is a forum-sticky that lists some good ones). To get an idea of how to do things

[zyky]

Ops, yes, the 'new' and 'delete' commands. I forgot about them.
I was actually trying to write it in C, but Dev-C++ was always saying (using Vista):
[Build Error] [main.o] Error 1
for the simplest programs such as:

#include <stdio.h>
#include <stdlib.h>

int main(int argc, char *argv[])
{
  
  system("PAUSE");	
  return 0;
}

so I switched to C++ (but without using objects). I'm just doing some Algorithm exercises. With quick sort, heap sort, merge sort, stack and queue it worked fine but now I'm having this annoying problem. I'll read some of my C++ book again to see if I'm doing something wrong.

[Fbody]

When your program hits the system("PAUSE") line, does it actually pause or does it blow right through it, without pausing?

[thelamb]

Yeah reading up on C++ is probably the best way. And please forget about using system("PAUSE") (there are sticky's on this forum explaining why and how to do it correctly).

[zyky]

When your program hits the system("PAUSE") line, does it actually pause or does it blow right through it, without pausing?

If this line

// IF I TAKE THIS OUT, IT CRASHES
system("PAUSE");

is compiled, I have to press a key to let the program go on, then it correctly runs the following line

printf("Minimo = %d.\n", minimo(l));

and works as expected.

Well, now I have found the line that causes the problem. It's:

125.    free(&r);

If I rewrite the minimo (minimum) function like this:

int minimo(struct Lista* l) {
    // se è vuota, restituisce un puntatore il valore massimo per un intero.
    if(isEmpty(l))
        return INT_MAX;

    int min = INT_MAX;
        
    // memorizza la testa della lista in un nuovo puntatore
    struct Record* r = l->testa;
    // per tutti gli elementi, memorizza quello con valore minore
    do {
        if(r->val < min)
            min = r->val;
    } while( (r = r->next) != NULL);

    return min;    
}

it works.

I'm not getting the reason why 'system("PAUSE")' changes its behaviour.

Also, if I want to free the memory where a pointer is allocated, is this code wrong?

int* pointer = (int*) malloc(sizeof(int))
free(&pointer)

If I use

free(pointer)

don't do I delete the memory location where the pointer points to? (instead of the memory location where the pointer is stored?)

[Fbody]

Using free(pointer) de-allocates the memory allocated by the malloc statement. It does not destroy the variable named pointer itself. That variable is only destroyed when it goes out of scope. I don't think there is a way to destroy a non-dynamic variable before it goes out of scope.

An example using C++ new/delete:

//global scope

void someFunc() { //begin someFunc() local scope
  int *pInt;  //create a local int pointer
  pInt = new int(5); //dynamically allocate an integer whose initial value is 5

  /* ...other operations... */

  delete pInt;  //dynamically de-allocate memory used by the integer pointed by pInt
} //end someFunc() local scope

//return to global scope, lifetime of integer pointer pInt automatically expires

[zyky]

Ok, I see. Thank you for the new/delete example.

I Think I was doing something else wrong. I was using:

struct Record* r = (struct Record*) malloc(sizeof(struct Record));

but I meant to place in there only a pointer to a Record, so I should have used:

struct Record* r = (struct Record*) malloc(sizeof(void*));

Also, I was thinking I created a pointer to a pointer, but now I remembered that a pointer to a pointer must be declared as 'Record**'. For example:

#include <cstdlib>
#include <iostream>

using namespace std;

int main(int argc, char *argv[]) {
    // int value
    int n = 123;
    
    // pointer to int value
    int* i = (int*) malloc(sizeof(void*));
    *i = n;
    printf("%d.\n", *i);                  // prints 123

    // pointer to pointer to int value
    int** j = (int**) malloc(sizeof(void*));
    *j = i;
    printf("%d.\n", **j);                 // prints 123
    
    // now free both pointer using 'j' only
    free(*j);
    printf("%d.\n", *i);                  // prints an unknown value
    free(j);
    printf("%d.\n", **j);                 // prints an unknown value
    
    getchar();
    return EXIT_SUCCESS;
}

I'm still wondering why the first code I posted worked fine with the system("PAUSE") line (or with getchar()) and crashed without it.
ps: yes I know about the system("PAUSE") <- getchar() if that's what you meant.