-1

I have written some code to help me deliminate dashes in my program for a larger linked list but when I call the class my code gets stuck on the deliminator loop and does not die. I cant even kill it with the kill command I have to open a new ssh client

int  deliminator(char word[], struct node *root){
    struct node *start =  (struct node*) malloc(sizeof(struct node));
    struct node *trav = (struct node*) malloc(sizeof(struct node));
    start->next= trav;
    trav->prev = start;

    char *token;
    token = strtok(word,"-");

    while(token){
            /* this loop is broken */

            printf("%s",token);
            struct node *curr  = (struct node*) malloc(sizeof(struct node));
            strcpy(curr->set, token);
            trav->next = curr;
            curr->prev = trav;
            trav = trav->next;

            token = strtok(NULL,"-");
    };
    root->next = start;
    return(0);


};

Also when I try to run the strtok improperly by looping with token = strtok(token,"-"); it gets stuck on the first token. I can't seem to find the problem, a friend of mine suggested that it had to do with the linked list nodes but i removed them and I had the same issue.

I call the deliminator class in this code snippit.

int main(int argc, char *argv[]){
    struct node *root = (struct node*) malloc(sizeof(struct node));
    struct node *trav = (struct node*) malloc(sizeof(struct node));
    root->next = trav;
    if(argc == 2){
    /*only one giant string*/
    deliminator(argv[1],root);
    while(root->next!= NULL){
    printf("%s",root->set);
    };
jay patel
  • 43
  • 5
  • 1
    What is `curr->set`? Are you allocating memory for it before copying into it? – Kon Feb 06 '19 at 20:51
  • 2
    please show the part of the program where you call `deliminator`, especially type and initializion of `word`; Also show the definition of `struct node`. – Stephan Lechner Feb 06 '19 at 20:56
  • I am allocating the memory before copying into it, the memory is allocated in the struct itself. – jay patel Feb 06 '19 at 21:07
  • 2
    Your loop `while(root->next!= NULL){ printf("%s",root->set); };` never changes `root` — you need something equivalent to `root = root->next;` somewhere in the loop, worrying about whether it is OK to change `root` itself (it probably isn't — you need it to free up the memory). – Jonathan Leffler Feb 06 '19 at 21:08
  • 2
    Additionally, you never set *any* node's `next` pointer to null, and `malloc()` cannot safely be relied upon to do that for you. You need to explicitly set the `next` (and `prev`) pointers null where that's the value those should have. – John Bollinger Feb 06 '19 at 21:10
  • Is `struct node *start` intentionally left empty and uninitialized (except for the `next` pointer)? – David C. Rankin Feb 06 '19 at 21:16
  • i see thanks @JohnBollinger the problem still persists but thanks for the tip, also thank you Johnathon leffler, didnt even notice that, guess im getting a little sloppy. – jay patel Feb 06 '19 at 21:17
  • @DavidC.Rankin yes it is, its so i can set root = to that start, I didn't do that just yet, I am still messing with some other stuff before I do that. – jay patel Feb 06 '19 at 21:19
  • How is `struct node` defined? – KamilCuk Feb 06 '19 at 21:27
  • Possible duplicate: [What is a debugger and how can it help me diagnose problems?](https://stackoverflow.com/questions/25385173/what-is-a-debugger-and-how-can-it-help-me-diagnose-problems) – n. m. could be an AI Feb 06 '19 at 21:27
  • 1
    Your `deliminator` function creates two `struct node` objects without assigning anything to their `set` members, and inserts these into the list ahead of any nodes into which you copy tokens. When you subsequently print the `set` members of these nodes in `main()`, you invoke undefined behavior. – John Bollinger Feb 06 '19 at 21:33
  • 1
    Overall, no, you are not having an issue with `strtok()`. The usage of that function in your code is just fine. You do, however, have an impressive variety of issues with details of setting up, maintaining, and examining the contents of your linked list. I therefore suggest running your program under Valgrind or a similar memory-use checker. Valgrind ought to recognize all of the specific issues I noticed. – John Bollinger Feb 06 '19 at 21:50

1 Answers1

1

Your code is mostly structured OK, and uses strtok correctly. But you don't initialize your variables or the fields inside the allocated nodes. I switched your call from strcpy to strdup so you allocate memory, and used calloc instead of malloc so that the pointers are initialized to null. In deliminator, you only need to allocate the nodes inside the loop, and just keep a pointer trav to traverse the list, and you can leave the root node alone.

I left if to you to figure out how to not waste the memory on a root node, which you don't really need. You should just have a root pointer, and pass in its address to deliminator. Also, you should clean up and free the nodes and allocated strings from strdup before you exit.

int  deliminator(char word[], struct node *root) {
            struct node *trav = root;

            char *token;
            token = strtok(word,"-");

            while(token){
                            /* this loop is fixed! */

                            printf("DEBUG: %s\n",token);
                            struct node *curr  = calloc(1, sizeof(struct node));
                            curr->set = strdup(token);
                            trav->next = curr;
                            curr->prev = trav;
                            trav = trav->next;

                            token = strtok(NULL,"-");
            };
            return(0);


}

int main(int argc, char *argv[]){
            struct node *root = calloc(1, sizeof(struct node));
            if(argc == 2){
                            /*only one giant string*/
                            deliminator(argv[1],root);
                            root = root-> next;
                            while(root != NULL){
                                            printf("%s\n",root->set);
                                            root = root->next;
                            }
            }
}
bruceg
  • 2,433
  • 1
  • 22
  • 29