views:

930

answers:

5

As a part of an assignment, I need to write two functions:

  1. a function that sums up two natural numbers represented as a linked list
  2. a functions that prints a number represented in the same way.

for some reason, both function work perfectly fine separately, but when I try to use the print function on the result of the sum function, it changes the value of the sum right in the beginning of the print function , and prints the wrong value. when I use printf to print the same value in the main, there is no problem. my code is detailed below. any ideas?

void main() 
{
  int a[1] = { 1 },
    b[1] = { 2 };
  int * *pa, **pb;
  List lst1, lst2;
  List sum;

  pa = (int * *) malloc(sizeof(int * )); * pa = &a[0];
  pb = (int * *) malloc(sizeof(int * )); * pb = &b[0];
  lst1 = arrToList(pa, 1);
  lst2 = arrToList(pb, 1);
  addNumbers(lst1, lst2, &sum);
  //printf("%d\n",*(sum.head->dataPtr));
  printNumber(sum);
}

//a function that recieves a number represented ad a list and prints it
void printNumber(List num) 
{
  ListNode * curr;
  int currData,
  i,
  number;

  if (isEmptyList(num) == TRUE) 
    printf("the input was an empty list, nothing to print");
  else 
  {
    i = 0;
    number = 0;
    curr = num.head;
    while (curr != NULL) 
    {
      currData = *(curr - >dataPtr);
      number = number + currData * ((int) pow(10, i));
      curr = curr - >next;
      i++;
    }
    printf("%d \n", number);
  }
}

// a function that sums in list 
// representation two numbers,
// each represented as a list 
void addNumbers(List n1, List n2, List * sum) 
{
  ListNode * currN1;
  ListNode * currN2;
  ListNode * currSum;
  int currN1N2Sum; //stores the sum of the current digits in n1 and n2 
  int carrier,
  prevCarrier; //current and previous  carriers that carries +1 to the 
  next digit of sum
  if the lst sum was bigger then 9

  if ((isEmptyList(n1) == TRUE) || (isEmptyList(n2) == TRUE)) 
    printf("bad input =(");
  else 
  {
    currN1 = n1.head;
    currN2 = n2.head; * sum = createEmptyList();
    carrier = 0;
    prevCarrier = 0;
    while ((currN1 != NULL) && (currN2 != NULL)) 
    {
      currN1N2Sum = *(currN1->dataPtr) + *(currN2->dataPtr) + prevCarrier;
      if (currN1N2Sum > 9) 
      {
        carrier = 1;
        currN1N2Sum = currN1N2Sum - 10;
      }
      currSum = creatNewListNode( & currN1N2Sum, NULL);
      insertNodeToEnd(sum, currSum);
      prevCarrier = carrier;
      carrier = 0;
      currN1 = currN1 - >next;
      currN2 = currN2 - >next;
    } //while ((currL1!=NULL)&&(currL2!=NULL))

    while (currN1 != NULL) 
    {
      currN1N2Sum = *(currN1 - >dataPtr) + prevCarrier;
      currN1 = currN1 - >next;
      if (prevCarrier != 0) prevCarrier = 0;
    }

    while (currN2 != NULL) 
    {
      currN1N2Sum = *(currN2 - >dataPtr) + prevCarrier;
      currN2 = currN2 - >next;
      if (prevCarrier != 0) prevCarrier = 0;
    }
  } // ! ((isEmptyList(n1)==TRUE)||(isEmptyList(n2)==TRUE))
}

here is the rest of the code:

typedef struct listNode{
int* dataPtr;
struct listNode* next;
} ListNode;

typedef struct list
{
ListNode* head;
ListNode* tail;
} List;

List createEmptyList()//creates and returns an empty linked list 
{
    List res;

    res.head = res.tail = NULL;

    return res;
}

Bool isEmptyList ( List lst )//checks if a given list is empty or not
{
    if (lst.head == NULL && lst.tail == NULL)
     return TRUE;
    else
     return FALSE;
}

void insertDataToEnd ( List * lst, int *dataPtr ) //inserts new data to the end of an existing linked list
{
    ListNode * newTail;
    newTail = creatNewListNode ( dataPtr, NULL );
    insertNodeToEnd(lst,newTail);
}

void insertNodeToEnd ( List * lst, ListNode * newTail )//insert an existing node to an existing linked list
{
    if (isEmptyList(*lst) == TRUE )
     insertNodeToStart ( lst,newTail );
    else
    {
     (*lst).tail -> next = newTail;
     newTail->next = NULL;
     (*lst).tail = newTail;
    }
}


ListNode * creatNewListNode ( int * dataPtr, ListNode * next )//inserts new node in an existing linked list
{
    ListNode * res;

    res = (ListNode *) malloc (sizeof(ListNode));

    res -> dataPtr = dataPtr;
    res -> next  = next;

    return res;
}

void insertNodeToStart  ( List * lst, ListNode * newHead )//inserts node to the begining of a given linked list
{
    if ( isEmptyList( *lst ) == TRUE )
    {
     (*lst).head = newHead;
     (*lst).tail = newHead;
     newHead -> next = NULL;
    }
    else
    {
     newHead -> next = (*lst).head;
     (*lst).head = newHead; 
    }
}
A: 

We need to see some more code: how you define the data structure for holding nodes, how you add nodes etc.

The following line is suspect:

    number=number+currData*((int)pow(10,i));

Say, you have 123 stored as 1, 2, and 3 nodes:

    number =  0;
    number =  0 + 1 * 1   = 1;
    number =  1 + 2 * 10  = 21;
    number = 21 + 3 * 100 = 321;

But if you store is as 3, 2, and 1 nodes you'd get:

    number =  0;
    number =  0 + 3 * 1   = 3;
    number =  3 + 2 * 10  = 23;
    number = 23 + 1 * 100 = 123;

Is this your problem?

dirkgently
The thing is, by debugging I know that the value is lost right in the beginning of the function, even before the line: if (isEmptyList(num)==TRUE)
Lior
Show us the definition of List and for that matter the whole code.
dirkgently
A: 

I'm not if this is an issue or not without seeing the implementation of createNewListNode(), but here's something to think about:
Where are the dataPtrs in the sum list pointing after you return from the addNumbers() call?

AShelly
dataPtr points to 0x0012fdf4 after returning from addnumbers(). It points the same address when it enters the print function, BUT the value changes to -858993460 and currData, i and number have all the same value.
Lior
That's exactly what I suspected - you were keeping a pointer to local storage. I was trying to point you towards the answer @chmike gave without doing your homework. Learning to debug is a very valuable skill to develop.
AShelly
A: 
ParoXoN
Just tried it. the result remains the same =((and I can't change the prototype of the function, because they are part of the exercise)
Lior
+5  A: 

The bug is in the function addNumbers. When you add a node to store the sum you pass a pointer to the variable currN1N2Sum which is a local variable (stored on the stack). When the addNumbers function terminates, the storage of the local variable is set free. The value found at that location will remain unchanged and thus apparently valid as long as the storage is not reused.

This is why you had the impression the addNumbers function was correct. When calling the printNumber function the storage is overwritten and you find a different value in there.

This explain your bug.

There is another problem with addNumbers. When you will try to add two digit numbers, the content of the currN1N2Sum will be overwritten by a new value.

What you should do is allocate a buffer (malloc) and store the value contained into currN1N2Sum into it. Pass the pointer to the buffer into the new node.

BTW: you may change (*lst).head in lst->head. It will make your code more readable.

chmike
Yes! that was the problem! I fixed it and it worked! thank you very very much =)
Lior
A: 

You've got a problem with createEmptyList. you declare there a list called res and return the structure but the minute this function returns that structure is not valid any more. try using malloc (for the struct) and then return the pointer to the caller. You use this function in the beginning with *sum.

This is a similar bug to what chmike found so you better fix both.

Dror Cohen