Please Help with C threads

[jerryR]

Ok, I originally thought the problem was with the threads but now i suspect sscanf, though i have no clue as to why, maybe someone can shed some light on the highlighted lines and the output below:

Code:

typedef struct sItem {
  struct sItem *nextItem;
  struct sItem *prevItem;
  char *symbol;
  int value;
} stockItem;

stockItem *stockList = NULL;

int main (int argc, char *argv[]){
  stockItem *stock;
  char userInput[128];
  char first[64];
  int value;
  while (1){
    printf ("Enter> ");
    if (fgets (userInput, sizeof(userInput), stdin) == NULL){
      exit (0);
    }
    stock = stockList;
    if (stock!= NULL){printf("\nSTOCK1:%s %d\n",stock->symbol,stock->value);}
    if (sscanf(userInput, "%s %d", first, &value) == 2){
      if (stock!= NULL){printf("\nSTOCK2:%s %d\n",stock->symbol,stock->value);}
        insertStock(stock, first, value);
      }
      stock = stockList;
      while (stock != NULL){
        printf("\n%s %d\n", stock->symbol, stock->value);
        stock = stock->nextItem;
      }
   }
}

OUTPUT:

Enter > A 1

A 1
Enter > B 2

STOCK1:A 1

STOCK2:B 1

B 1
Enter > C 3

STOCK1:B 1

STOCK2:C 1

C 1

C 3
Enter > ctrl+D

Does anyone know why the value of stockList->symbol changes after sscanf???

[cwr]

You managed to use code tags, so why didn't you manage to indent your code?

People are far more likely to want to help if you:

(a) make the effort to make your code indented (and hence readable)
(b) make the code compilable, without them having to worry about the fact that you haven't supplied functions like errno_abort and err_abort.

Of course, they'd also need a implementation of POSIX threads installed.

[jerryR]

OK, reposted with indent and this code can be complied. Just cant seem to see where stockList->symbol is being changed after its being read from the command line...!?!

[bithub]

  stockItem *stock;
  while (1){
    pthread_mutex_lock(&stockMutex);
    if (stock == NULL){
      stock = stockList;
    } else {              	
      stock = stock->nextItem;
    }

There is a problem here. Here you check if stock is NULL before it is every assigned any value at all. Chances are that stock is some random value which you then try and dereference after the NULL test fails. You should probably read up on some linkedlist tutorials as well, because that is the most complicated way of printing a list that I have ever seen.

[jerryR]

Thanks for the reply! I check if stock is NULL because I assume that the during the first iteration it is NULL, and after iterating through the list, it'll eventually come to be NULL as stock->nextItem == NULL means the end of the list and so its assigned to the top of the list. I think my problem is pointers, more so than linked lists, but the printing is complex due to thespecifications, but I would appreciate your advice on making it simpler. Thanks again.

[bithub]

because I assume that the during the first iteration it is NULL

How can you assume that? You've done the equivalent of:

int *i;
if(i == NULL)
   //...

[jerryR]

OK, I hope adding "= NULL" to "stockItem *stock = NULL" will help, but still not sure where stockList->symbol is being updated after adding 2nd, 3rd...etc items.

[bithub]

if (stock == NULL){
    stockList = toInsert;
  }

You never set values for toInsert->prevItem and toInsert->nextItem.

      } else {
        if (stock->prevItem == NULL){
          stock->prevItem = toInsert;
	  toInsert->nextItem = stock;	
	} else if (stock->nextItem == NULL){
          stock->nextItem = toInsert;
	  toInsert->prevItem = stock;	
	} else {	
          stock->prevItem->nextItem = toInsert;
	  toInsert->prevItem = stock->prevItem;
	  stock->prevItem = toInsert;
	  toInsert->nextItem = stock;	
	}

Here you need to break out of the while loop once you have inserted the new node.

        if (stock->prevItem == NULL){
          stock->prevItem = toInsert;
	  toInsert->nextItem = stock;	
	}

toInsert->prevItem should be set to NULL here, right?

	} else if (stock->nextItem == NULL){
          stock->nextItem = toInsert;
	  toInsert->prevItem = stock;	
	}

toInsert->nextItem should probably be set to something here... I'll let you figure that out.

[jerryR]

Thank you for your help! Is there a reason why an object that's not initialized isnt automatically set to NULL in C? default values?? Anyways, after applying your changes I get this output:

Enter > A 1
A 1
Enter > B 2
B 1

B 1

so, still not doing what im trying to get it to do. It looks like something in the 2nd thread is changing the stockItem->symbol value but I dont see where/how this happens...

[bithub]

First of all, you should never edit your original post like that. Now people that view this thread will be completely lost at the responses since they have nothing to do with the original question.

Is there a reason why an object that's not initialized isnt automatically set to NULL in C?

None that I can think of, but I can't really think of a good reason to automatically set pointers to NULL either.

Looking at your new code, I dont see where userInput is ever filled with with anything before your sscanf() call.

[jerryR]

I'm sorry, i shoulnt have posted the whole code to begin with as there were several other problems, but im trying to focus in on this one problem i cant get past, and i left out one line when reposting (i left out many lines cuz i jsut wanted to post the bare bones) that has fgets (userInput, sizeof(userInput), stdin) so that is where userInput is reading the user's input...any reason why sscanf changes the value of a pointer variable its not assigned to change??

[bithub]

your sscanf call is changing the value of stockList->symbol, but that's because stockList->symbol is just a pointer to a buffer. Look at it this way:

You have a buffer called first. When you call sscanf() first gets filled with some characters. Then in your insertStock() function you point the stockList->symbol pointer to first. Then you call sscanf() again, and fill the buffer with new characters. Remember though that stockList->symbol just points to first, so the characters it points to get changed as well.

One way to fix this is to change your structure definition to

typedef struct sItem {
  struct sItem *nextItem;
  struct sItem *prevItem;
  char symbol[32];
  int value;
} stockItem;

Then you strcpy() to copy the characters into stock->symbol.

[jerryR]

Thanks bithub..I really had no idea, I'm no good with pointers (nor C in general) and I had no clue this is what was happening!

Thanks again!