0

I created a program in C which reads words from the file and stores them to a linked list but I noticed that the second continue causes undefined behavior Why is this happened?

there are 3 functions

The first function creates the list which is fine

the second function fills the list with data

the third displays the content of the list

When I ran the program is invoked to undefined behavior

FILE: https://gist.github.com/up1047388/b3018bc2a1fb0d66e86855a0d54baf63

My code :

    #include<stdio.h>
    #include<stdlib.h>
    #include<string.h>
    typedef struct node {
        char str[50];
        struct node *next;
    }Node;
    
    void createList(Node ** head , int len )
    {   int i=0;
        Node **lpp ;
        Node *komvos ;
        Node *komvos1;
        komvos = (Node*)malloc(sizeof(Node));
        komvos -> next = NULL;
        lpp=&komvos;
        for(i=1 ; i < len ; i++)
        {
            komvos1 = (Node*)malloc(sizeof(Node));
            komvos1 -> next = NULL;
            (*lpp) -> next = komvos1;
            lpp = &(*lpp) -> next;
        }
        
        *head = komvos ;
    }
    
    void FileList(FILE *fp  , Node *head) 
    {   char c;
        char tempStr[50];
        char str[50];
        int i = 0 , j = 0;
        Node **lpp;
        lpp=&head;
        
    for(c=fgetc(fp) ; c!=EOF ; c=fgetc(fp))
    {
        str[j]=c;
        j++;
        }   
    str[j]='\0';    
    j=0;    
    
    while(str[j]!='\0')  
    {   
        
        if (str[j] == ' ')
        {
            if (i == 0)
            {
                continue;
            }
            
            tempStr[i] = '\0';
            i = 0;
            
            
             strcpy((*lpp) -> str , tempStr);
             
             lpp = &(*lpp)  -> next ;       
            
            //continue  //This continue caused the problem
        }
        
        tempStr[i] = str[j];
        i++;
        j++;
        
        
    }   
        
    }
    
    void printList(Node *head)
    {
        Node *temp;
        temp = head;
        for(;temp!=NULL;temp=temp->next)
        {
            printf("\nthe words are  : %s", temp -> str);
        }
        
    }
    
    
    int main ()
    {   
    
        Node *head ;
        head = NULL;
        FILE *fp;
        fp = fopen ("lists4.txt","r+");
        if (fp==NULL)
        {
            printf("the file is broken");
            exit(8);
        }
        
        createList(&head , 3);
        FileList(fp,head);
        printList(head);
        
        return 0;
}
vg-png
  • 9
  • 2
  • 1
    Why are you providing an external link for only one line? Paste it directly in your question, please. – MikeCAT Jun 16 '21 at 15:29
  • 2
    The commented `continue` will turn the loop into an inifinite one because `j` is not updated before that. – MikeCAT Jun 16 '21 at 15:32
  • @MikeCAT isn't the same true of the first `continue`, assuming it ever executes? – Dave Costa Jun 16 '21 at 15:33
  • @MikeCAT It was the file – vg-png Jun 16 '21 at 15:33
  • 3
    I'm curious: What do you mean by "causes undefined behavior"? Did you get an error message with those words in it? (I know what undefined behavior is, but it's unusual to see it in a question like this. It usually shows up in the answers.) – Steve Summit Jun 16 '21 at 15:36
  • Yes, the first `continue;` will cause an infinite loop (without the second one) if the first character in the file is `' '`. Without the second `continue`, it won't get executed except for the first iteration because `i` is not zero (thanks to `i++;`) – MikeCAT Jun 16 '21 at 15:36
  • This would be a very good time to learn how to use a *debugger* to step through your code statement by statement while monitoring variables and their values. – Some programmer dude Jun 16 '21 at 15:39
  • @SteveSummit I thought that when I ran a program and in the terminal it does not print anything this is undefined behavior – vg-png Jun 16 '21 at 15:41
  • @vg-png That's not UB. See this: https://stackoverflow.com/q/2397984/6699433 – klutt Jun 16 '21 at 15:47
  • @klutt Ok but what is the type of this behavior? – vg-png Jun 16 '21 at 15:48
  • 1
    @vg-png Logical error probably. It's nothing wrong per se with an endless loop. – klutt Jun 16 '21 at 15:50
  • And I'm not sure what you're trying to do here. Why do you want to add that continue statement? – klutt Jun 16 '21 at 15:51
  • 1
    @vg-png Undefined behavior is a specific reason -- actually, a whole class of reasons -- for a program not to work. Usually when a program doesn't work as expected, we just start out by saying "it doesn't work". :-) In your case, there probably is some UB involved, but a program that runs forever and printed no output could simply be suffering from a logic error, as klutt suggested, or maybe just a missing `printf` call. – Steve Summit Jun 16 '21 at 15:54
  • 3
    There's no UB here, at least not with that `continue;` - It is *defined behaviour* to endlessly loop if you're trying to iterate over a string and not increment the index. – tofro Jun 16 '21 at 16:38
  • @vg-png One question. Do you understand why the following is an endless loop? `int i=10; while(i>0) { continue; i--; }` – klutt Jun 16 '21 at 16:54
  • @klutt yes because continue preceed from i--; it creates an infinite loop – vg-png Jun 16 '21 at 17:01
  • @vg-png And that's exactly what that `continue` does in your code. – klutt Jun 16 '21 at 17:03
  • @klutt I used continue because I wanted to not have space inside the striings which I store in my list – vg-png Jun 16 '21 at 17:04
  • regaarding: `printf("the file is broken");` this is not true. Strongly suggest using `perror()` so your error message and the text reason the system thinks the error occurred are output to `stderr` – user3629249 Jun 18 '21 at 14:32
  • regarding: `char c;` and `for(c=fgetc(fp) ; c!=EOF ; c=fgetc(fp))` the function: `fgetc()` returns an `int`, not a `char`. And generally, a `char` cannot represent a EOF. – user3629249 Jun 18 '21 at 14:37
  • OT: regarding: `komvos1 = (Node*)malloc(sizeof(Node));` 1) in C, the returned type is `void*` which can be assigned to any pointer. Casting just clutters the code (and is error prone) 2) always check (!=NULL) the returned value to assure the operation was successful. – user3629249 Jun 18 '21 at 14:40

1 Answers1

0

The commented out continue will make the loop run infinitely because it prevents j from being updated.

To avoid this, add code to update j before the continue in the str[j] == ' ' case.

    while(str[j]!='\0')
    {
        
        if (str[j] == ' ')
        {
            j++; /* add this to update j */
            if (i == 0)
            {
                continue;
            }
            
            tempStr[i] = '\0';
            i = 0;
            
            strcpy((*lpp) -> str , tempStr);
            
            lpp = &(*lpp)  -> next ;
            
            continue;  /* and then turn on the continue */
        }
        
        tempStr[i] = str[j];
        i++;
        j++;
        
    }

I'll prefer using if-else in this case.

    while(str[j]!='\0')
    {
        
        if (str[j] == ' ')
        {
            j++;
            if (i == 0)
            {
                continue;
            }
            
            tempStr[i] = '\0';
            i = 0;
            
            strcpy((*lpp) -> str , tempStr);
            
            lpp = &(*lpp)  -> next ;
        }
        else
        {
            tempStr[i] = str[j];
            i++;
            j++;
        }
        
    }
MikeCAT
  • 73,922
  • 11
  • 45
  • 70
  • In the first version you add `j++` to update it I understand this But in second version why you use a j++; (There is not the continue which caused the problem) – vg-png Jun 16 '21 at 15:47
  • @vg-png `j++;` is used for exactly same reason: to have it move to the next element for processing. – MikeCAT Jun 16 '21 at 15:51
  • It might be even better as a `for` loop, with `j++` as the iteration expression. That way `j++` is executed at the end of every iteration, whether it finished via `continue;` or by reaching the end of the block, and you don't have to duplicate it in both paths. – Nate Eldredge Jun 16 '21 at 16:28