views:

110

answers:

2

I'm writing a program for a "Set" data structure in C.

It's working fine when one instance of Set is present, but it doesn't work when two instances of Set are present.

Also my code for removing an element is not working when the candidate is the first element.

Can someone help me?

Here is the code:

+5  A: 

First of all you passes pointer to set by value. I.e. instead of

void Set_Create (Set* set) {
     set = malloc(...); // Malloc here leads to memory-leak.
     //...
}

int main() {
     Set *set; // Some value like 0xdeadbeef pointing somewhere
     Set_Create (set);
     // set have still undefined value
}

use

Set *Set_Create () {
     Set *set = malloc(...);
     /// ...
     return set;
}

int main() {
     Set *set; // Some value like 0xdeadbeef pointing somewhere
     set = Set_Create ();
     // set point to new memory
}

or:

void Set_Create (Set *set) {
     // No malloc
     /// ...
     return set;
}

int main() {
     Set set; // Create on stack
     Set_Create (&set); // Initialize
     // ...
}

I'd advice re-reading chapter on pointers (don't worry - they are considered hard but they are must for C programmers).

Maciej Piechotka
+4  A: 

I suspect that you are asking us to help you with your homework, so I am not going to solve all of your code problems for you. I am only going to point out your most egregious mistake, which is the mistake you asked about -- why does the second call to your create_Set() function fail?

test.c, lines 27-28:

   Set* s2;
   Set_Create(s2); //.. Here error comes..

In set.c, I have trimmed it down to the meaningful code:

void Set_Create(Set* set){
   if(set == NULL){
       set = (Set *)malloc(sizeof(Set));
   }

   set->size = 0;
   set->data = -1;
   set->next = NULL;
}

What's happening is that both of your calls to Set_Create are overwriting some arbitrary, unpredictable, memory locations. This is a serious bug.

In the case of set1, the arbitrary pointer you have been experiencing seems to happen to point to some place in memory which can be written to. Who knows what data you are clobbering, but your data can survive in that spot for a short duration.

In the case of set2, the arbitrary pointer you are encountering is not a valid memory location for you to modify, so your program crashes.

Change your create_Set like so:

Set* Set_Create() {
   Set * set = calloc(1, sizeof(Set));

   set->data = -1;
   return set;
}

And in test.c:

Set * set1 = Set_Create();
Heath Hunnicutt