CODEWITHSUNDEEP
cstringparsingmemoryfault
Syed Rumman
Syed Rumman

Reputation: 99

Segmentation Fault when I add a for Loop

When I add the for loop I get segmentation fault. Also, when I add buffer[i] !='\0' in the while loop condition, I get segmentation fault error. I am having a hard time trying to understand why this error pops up. Thanks.

 #include <stdio.h>
 #include <string.h>
 #include <stdlib.h>

int main()
{
    char buffer[2000] = "-rw-r--r-- 1 ftp ftp 614400 Oct 18  2006\r\n"
    char String[2000];
    int i, j, k= 0;
    int nextline= 0;

   for(k = 0; k<strlen(buffer);k++)
    {
        while((buffer[i] != '\r' && buffer[i+1] != '\n'))
         {
           String[j] = buffer[i];
           i++;
           j++;
         }

   }
  printf("%s", String);               
}

Upvotes: 0

Views: 94

Answers (1)

AJNeufeld
AJNeufeld

Reputation: 8695

A loop of the form for(k=0; k < strlen(buffer); k++) { ... } is generally very bad code. It is O(n²), meaning that the time for the loop increases quadradically as n increases. Why? Each pass through the loop, the strlen function is called to determine the length of the string in buffer. If the string is 1000 character long, each strlen internally loops 1000 times, and it itself is called 1000 times, for 1000000 iterations of the inner loop! Instead, the length of the string should be calculated once, outside the loop. Eg)

int buffer_len = strlen(buffer);
for(k=0; k<buffer_len; k++) { ... }

You could also use a char * as your loop index, and loop until you encounter the null character:

for(char *c_ptr = buffer; *c_ptr != '\0'; *c_ptr++) { ... }

At any rate, for your problem, you do not need the double loop:

for(k = 0; k < strlen(buffer); k++)
{
    // ...
    while( /* incorrect condition here */ ) {
        // ...
    }
    // ...
}

The above suggests you want to loop through each character in your string, and then starting at each of those characters, perform another inner loop. What you probably want is just an if( ) statement:

for(k = 0; k < strlen(buffer); k++)
{
    // ...
    if( buffer[k] == '\r'  &&  buffer[k+1] == '\n' ) {
        // ...
    }
    // ...
}

I'll leave you to struggle with what goes in the // ... comments, if anything. You learn more by doing.

As others have pointed out, your i & j variables were left uninitialized. You will want to ensure you initialize them properly before using them. You did initialize k to zero, which was actually unnecessary since the for(k=0; ... ; ...) loop is already initializing the value of k.

Upvotes: 1

Related Questions