Thread: Confusion using pointers

  1. #1
    Registered User
    Join Date
    Mar 2010
    Posts
    6

    Confusion using pointers

    I just registered on this board for help with a problem I'm having. I recently had to edit an older assignment to use a linked list and after completion of the program it seg faults right off the bat. I understand the difficulties with trying to diagnose these but I really have no idea where to start, I've had a lot of problems with seg faults in the past and something about arrays or pointers or possibly using the wrong scanning function but in general I just must be missing something about these at least.

    Here's the beginning of my code, the rest is simple functions that work similarily to the add() function. The gist of the program is that it's supposed to open a file and read its contents into a linked list, then shoudl dynamically update the linked list based on what the user inputted and then rewrites the file at the end. The file is also ciphered using a simple cipher before being written to the file.

    Code:
    #include<stdlib.h>
    #include<stdio.h>
    #include<string.h>
    
    char* decipher(char* decipher);
    char* cipher(char* cipher);
    void menu(list* ptr);
    list* add(char* bar[], list* ptr);
    list* edit(char* bar[], list* ptr);
    list* verify(char* foo[], list* ptr);
    list* del(char* bar[], list* ptr);
    void addToList(list* ptr, char* foo, char* poo, char* boo);
    void printList(list* ptr);
    //prototypes
    
    #include"cipher.c"
    #include"menu.c"
    
    typedef struct list
            {
            char *user;
            char *pass;
            char *type;
            struct list *next;
    	 
            } list, node;
    
    
    
    list* initializeList() {
    
      char buffer[50];
      node* initial;
      node* ptr;
      FILE* addUser = fopen("password.csv", "rt");
    
      initial = (node*)malloc(sizeof(node));
      if (addUser != NULL) 
      {
        fgets(buffer, 50, addUser);      //Initializes first node of list
        initial->user = strtok(buffer, " ,");
        initial->pass = strtok(buffer, " ,");
        initial->type = strtok(buffer, "\n");
        initial->next = NULL;        //ready's addition of next node
        ptr = initial;
    
        while(!feof(addUser)) 
        {
          while(ptr->user != NULL)
                      ptr = ptr->next;
    
          node* link = (node*)malloc(sizeof(node));
          fgets(buffer, 50, addUser);
          link->user = strtok(buffer, " ,");
          link->pass = strtok(buffer, " ,");
          link->type = strtok(buffer, "\n");
          ptr->next = link;
    
        }
      }
      fclose(addUser);
      return initial;
    }
    
    
    
    int main(int argc, char* argv[]) {
    
    	printf("lolol");
        list* linkList = initializeList();
        char* choice = argv[1];
        int i;
    	
    
    	if (!strcmp("-menu", choice)){//If statement processes menu mode, if specified by user
    		menu(linkList);
    		printList(linkList);
    		free(linkList);
    		return (-1);
    	}
    	for (i = 2; i < argc; i++){
    			if (!strcmp(argv[i],"-menu"))//Determines if menu was incorrectly specified at command-line
    			{
    				printf("Syntax Error: -menu must be the first argument.\n");
    				return (-1);
    			}
    	}
    	
    	if (!strcmp(choice, "-add"))
    		add(argv, linkList);
    	else if (!strcmp(choice, "-del"))
    		del(argv, linkList);
    	else if (!strcmp(choice, "-edit"))
    		edit(argv, linkList);
    	else if (!strcmp(choice, "verify"))
    		verify(argv, linkList);
    	else
            {
    			printf("Syntax Error: possible arguments include -menu to enter menu-driven mode or,"
    			" -add, -del, -edit, or -verify followed by the entry arguments.\n");
    			return (-1);
    	}
    	
    	printList(linkList);
    	free(linkList);
    	return 0;
    }
    
    list* add(char* array[], list* linkList){
    
        node* ptr = linkList;
        char *userCheck;
        char *ciphered[3];
    
        printf("How about here");
    
        while (ptr->user != NULL) 
        {
          userCheck = strdup(ptr->user);
          userCheck = decipher(userCheck);
          if (!strcmp(array[2], userCheck)) 
          {
    	printf("Entry already exists");	//Checks if user is already present in the directory
    	return (-1);	      	//Does not allow for more than one instance of the same name.
          }
          ptr = ptr->next;
    
        }
        ciphered[0] = strdup(cipher(array[2]));//Passes user, password and type to be ciphered
        ciphered[1] = strdup(cipher(array[3]));
        ciphered[2] = strdup(cipher(array[4]));
    
        ptr = (node*)malloc(sizeof(node));  //creates new struct for contents to be added
        addToList(ptr, ciphered[0], ciphered[1], ciphered[2]);
        
        return linkList;
    }
    Any minor thoughts or things I could try would help as well.

  2. #2
    Registered User
    Join Date
    Sep 2007
    Posts
    1,012
    The first place that I see that is problematic is this:
    Code:
    while(ptr->user != NULL)
      ptr = ptr->next;
    I presume you mean: while(ptr->next != NULL): otherwise you won't be able to detect if you fall off the end of the list.

    In addition, I noticed that your usage of strtok() is not correct. The first argument to strtok() on the first call to strtok() should be your string; but subsequent calls (while tokenizing the same string) should have the first argument set to NULL. Something like:
    Code:
    initial->user = strtok(buffer, " ,");
    initial->pass = strtok(NULL, " ,");
    initial->type = strtok(NULL, "\n");
    ...
    link->user = strtok(buffer, " ,");
    link->pass = strtok(NULL, " ,");
    link->type = strtok(NULL, "\n");
    It's also a bad idea to loop on the return value of feof(). feof() is not predictive, it's reactive; so it will not return true (ie EOF) until after a read function has noticed EOF. Instead, you want something like:
    Code:
    while(fgets(buffer, sizeof buffer, fp) != NULL)
    That is, loop on the read function, because it will be happy to tell you when it found EOF. Looping on feof() will make it appear as though the last line of the file is read twice.

  3. #3
    Registered User
    Join Date
    Mar 2010
    Posts
    6
    I updated my code for all the things you mentioned but it still seg faults off the start.

  4. #4
    Registered User
    Join Date
    Sep 2007
    Posts
    1,012
    Your next step (which should always be your first step on a segfault) is to use a debugger. I recommend Valgrind if it supports your platform. If not, gdb probably does. These will help you pinpoint the problem.

    Valgrind is great because it can usually tell you where the problem occurred, as opposed to where the symptom (the segfault) is occurring.

  5. #5
    Registered User
    Join Date
    Mar 2010
    Posts
    6
    Thanks! I didn't know programs like these existed. I'll get right on it.

  6. #6
    Registered User
    Join Date
    Mar 2010
    Posts
    6
    I'm still having problems compiling this. I've tried using valgrind and I've pinpointed some errors but it's still not functioning correctly. I'm not sure what command/tool I should be using to pinpoint the problems in valgrind but I've been using memcheck, aside form that I'm not sure what else to do.

    If anyone can check out if I've defined everything properly and if I don't have any null pointers or anything to that effect that would be very helpful. I believe the problem most likely lies inside of my initializer for the linklist I'm creating but I'm still unsure as to the problem exactly.

    Code:
    list* initializeList() {
    
      char buffer[50];
      node* initial = (node*)malloc(sizeof(node));
      node* ptr = initial;
      FILE* addUser = fopen("password.csv", "rt");
    
      if (addUser != NULL) 
      {
        fgets(buffer, 50, addUser);      //Initializes first node of list
        initial->user = strtok(buffer, " ,");
        initial->pass = strtok(NULL, " ,");
        initial->type = strtok(NULL, "\n");
        initial->next = NULL;        //ready's addition of next node
        ptr = initial;
    
        while(fgets(buffer, 50, addUser) != NULL) 
        {
          while(ptr->next != NULL)
                      ptr = ptr->next;
    
          node* link = (node*)malloc(sizeof(node));
          fgets(buffer, 50, addUser);
          link->user = strtok(buffer, " ,");
          link->pass = strtok(NULL, " ,");
          link->type = strtok(NULL, "\n");
          ptr->next = link;
    
        }
       fclose(addUser);
      }
      return initial;
    }

  7. #7
    Registered User claudiu's Avatar
    Join Date
    Feb 2010
    Location
    London, United Kingdom
    Posts
    2,094
    If you want 50 characters in your buffer you would better allocate space for 50+1 (including the string terminator \0)

  8. #8
    Registered User
    Join Date
    Sep 2007
    Posts
    1,012
    I suspect you don't want to be calling fgets() inside your loop like that; you'll be ignoring lines. Once you have something like:
    Code:
    while(fgets(buffer, 50, addUser) != NULL)
    you need not call fgets() again. Each iteration of the loop will fill up "buffer" with (at most) 49 bytes from the file. By calling fgets() inside of the loop you're ignoring the fgets() call that's controlling the loop.

    There's another substantial error that I completely missed in your initial post. When you are using strtok(), the return value from strtok() is a pointer inside the buffer you're tokenizing. You're not actually getting copies of the token. Thus each time you read a line, your previous tokens will get garbled because they're simply pointing inside of your single buffer. What's more, the array they point to is local to the function, so when it returns, any pointers to it become invalid. Your linked list entries all contain pointers to this (soon to be) invalid buffer, and that can cause problems; problems that even the mighty Valgrind might not be able to notice. You can solve this in a few ways. You might make user, pass, and type arrays and snprintf() the values to them; or you might use malloc() + strcpy() (or strdup() if you're targeting unix-like systems). This latter option would require a lot of free() calls if you want to avoid leaks.

    You'll also want to set link->next to NULL. Otherwise it'll contain garbage.

    As for valgrind, memcheck is the tool you'll want to focus on for now. The other tools are great, too, but for debugging memcheck is the way to go. When you're debugging, you always want to build with the -g flag (tells the compiler to include debugging symbols). Off the top of my head, the following are common memcheck errors:

    invalid read of size n: you tried to access memory that you're not allowed to. If you tried to read an int from an invalid pointer, for example, it will (probably) tell you that you had an invalid read of 4 bytes (4 bytes being a typical size for int).

    invalid write of size n: the same as the above but for storing, not retrieving

    conditional move depends on uninitialized value / syscall points to uninitialized bytes / use of uninitialized value of size n: you tried to use something before giving it a value

    There are probably more, but these will cover a lot of the issues you'll run into.

  9. #9
    Registered User
    Join Date
    Mar 2010
    Posts
    6
    Very informative post thanks a lot Cas. I'm a little confused as to how I can solve the problem of the local structure within loop. If I only need to create a new struct on the condition that there are more lines to be read, how do I go about doing this within the iteration of the loop?

  10. #10
    Registered User
    Join Date
    Sep 2007
    Posts
    1,012
    There's no problem with a local struct (you have no local structs; they're all allocated). And those you are creating each iteration, since you're calling malloc() each time.

    The problem is your array of char. It is local to the function. But that's OK, really, because you'll have to solve another problem first (the strtok() issue I mentioned), and once that's solved, the fact that "buffer" is local to the function doesn't matter. Example:
    Code:
    char *f(void)
    {
      char s[] = "foo";
      return s; /* bad because you're returning a pointer to storage that is local to the function, storage which ceases to exist when the function returns */
    }
    char *f(void)
    {
      char s[] = "foo";
      char *p = malloc(strlen(s) + 1);
      strcpy(p, s);
      return p; /* fine because you're returning a pointer to allocated storage, which exists until you call free() */
    }
    The fact that strtok() does not make copies of the tokens means you will have to make copies. Thus you'll be doing something more like the second function in the example above.

  11. #11
    Registered User
    Join Date
    Mar 2010
    Posts
    6
    Thanks for all the help cas, with yours and another c whiz friend of mine I was able to get it running. He said you had the right idea and he helped me implement it correctly, thanks again!

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. pointers to arrays
    By rakeshkool27 in forum C Programming
    Replies: 1
    Last Post: 01-24-2010, 07:28 AM
  2. sorting number
    By Leslie in forum C Programming
    Replies: 8
    Last Post: 05-20-2009, 04:23 AM
  3. Arrays, pointers and strings
    By Apropos in forum C++ Programming
    Replies: 12
    Last Post: 03-21-2005, 11:25 PM
  4. pointers
    By InvariantLoop in forum C Programming
    Replies: 13
    Last Post: 02-04-2005, 09:32 AM
  5. pointers, functions, parameters
    By sballew in forum C Programming
    Replies: 3
    Last Post: 11-11-2001, 10:33 PM