Segmentation fault after typing in text when prompted. HELP!

Question

Can you guys analyze my code. I am pure beginner so I am not really sure if I am doing this right. I want to know why am I receiving segmentation fault and if this code is in any way correct. I performed cs50 debugger and it showed me that segmentation fault is caused at isalpha function, but I do not know how to fix it, HELP! Instead of s I wrote text just for some personal clarification, but forgot to screenshot again.

#include <cs50.h> #include <stdio.h> #include <ctype.h> #include <string.h> #include <math.h>

int count_letters(string s);

int main(void) {

string text = get_string("Text: "); printf("Number of letters: %i", count_letters(text));

}

int count_letters(string text) { int n = 0; int i = 0;

for(i=0; (n = strlen(text)); i++) { if(isalpha(text[i])) { n++; } } return n; }

Answer 1

The problem actually lies in the for loop statement.

for(i=0; (n=strlen(s)); i++)

This has created an infinite loop and eventually the code overruns the end of string s. I'll try to explain.

A for loop statement has three components - the initialization, the limit test, and the increment component. They are seperated by semicolons. While the initialization and increment clauses are correct, your test clause is bad. The code is going to check whether the test clause is true to decide if another pass through the loop should be executed.

In this case, the test clause, (n=strlen(s)) is always going to be true, because an assignment statement always returns a value of true. (Note: a single = sign is an assignment statement while a double equals sign, or == is a logical test of whether two values are actually equal.) So, since the test clause is an assignment and always returns true, the result is an infinite loop. Eventually the code goes way past the end of the string, evaluating random memory until it hits something that it shouldn't access. This causes the seg fault.

In this case, it looks like you've put part of your initialization clause in your test clause while not adding an actual test. I believe what you want is this:

for(i=0, n=strlen(s) ;    i < n    ;  i++)  //extra spacing for clarity

Note that the test is less than, not equal to. The test must be true before each pass for the code to continue to loop. If it were i==n, the test would fail on the first pass because i is 0 and n is greater than 0.

Also note that strlen() is called once, during the setup of the for loop, rather than being part of the test clause. There's a best practice reason for this. When using strlen() in a for loop, it's best to NOT do it in the test because sometimes the string will have a character changed to the End of String character or \0. If this happens, the strlen() return value will actually change. So, it's best to set it once in the initialization and save it in a var - n in this case. It's also more efficient because the code won't have to rerun strlen on every loop.

Any questions? ;-)

If this answers your question, please click on the check mark to accept. Let's keep up on forum maintenance. ;-)