I'm a new person who loves to play around with coding. Recently I was going through a course on edx, and one of the exercises I need to complete has this small code snippet that keeps on giving Segmentation fault. I have taken out the faulty bit (everything else compiles nicely)
#include <stdio.h>
#include <string.h>
#include <cs50.h>
#include <ctype.h>
#include <stdlib.h>
int main (int argc, string argv[])
{
if (argc == 2 && isalpha(argv[1]))
{
int a = 0;
while (argv[1][a] == '\0')
{
a++;
printf("%c\n", argv[1][a]);
}
}
else
{
printf("Usage: ./programname 1-alphabetical word\n");
return 1;
}
}
The problem seems to be here: argv[1][a] but I can't for the life of me find out what, and how to fix it.
(1) isalpha(argv[1]) is wrong. This function expects a single character, but you are passing a pointer-to-string. That will certainly not give you any kind of expected result, and it's probably Undefined Behaviour into the bargain. You either need to loop and check each character, use a more high-level library function to check the entire string, or - as a quick and probably sense-changing fix - just check the 1st character as BLUEPIXY suggested: isalpha( argv[1][0] ) or isalpha( *argv[0] ).
(2) Your while condition is wrong. You are telling it to loop while the current character is NUL. This will do nothing for non-empty strings and hit the next problem #3 for an empty one. You presumably meant while (argv[1][a] != '\0'), i.e. to loop only until a NUL byte is reached.
(3) You increment index a before trying to printf() it. This will index out of range right now, if the input string is empty, as the body executes and then you immediately index beyond the terminating NUL. Even if the loop condition was fixed, you would miss the 1st character and then print the terminating NUL, neither of which make sense. You should only increment a once you have verified it is in-range and done what you need to do with it. So, printf() it, then increment it.
2 and 3 seem most easily soluble by using a for loop instead of manually splitting up the initialisation, testing, and incrementing of the loop variable. You should also use the correct type for indexing: in case you wanted to print a string with millions or billions of characters, an int is not wide enough, and it's just not good style. So:
#include <stddef.h> /* size_t */
for (size_t a = 0; argv[1][a] != '\0'; ++a) {
printf("%c\n", argv[1][a]);
}