KM1 KM1 - 3 months ago 12
C Question

Segmentation fault (Core dumped) - simple

I'm a beginner in C. I got a simple program where I want to change every keyword into a corresponding number . For instance A = 0, B = 1 and F = 5 etc. In this case keyword "hello" will be "7 4 11 11 14".
I can compile this code, but whenever I run this I get the error "Segmentation fault (core dumped)". I tried several things to change it, but without avail. Can someone please check my code and give me feedback? Any constructive feedback on my style, logic and other code related stuff are welcome as well!

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

int main(int argc, string argv[])
{

int keylength = strlen(argv[1]);
char *key = argv[1];

// insert keyword
if (argc != 2)
{
printf("Less commands please.");
return 1;
}
else
{
if (!isalpha(argv[1]))
{
printf("Please no numbers or weird symbols");
}
else
{
for (int i = 0; i < keylength; i++)
{
if(isupper(key[i]))
{
key[i] = key[i] - 65;
}

else if(islower(key[i]))
{
key[i] = key[i] - 97;
}
}
}
}

}

Answer

You could do it like

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

int main(int argc, char* argv[])
{
    if (argc != 2)
    {
        printf("Must provide one argument\n");
        return 1;
    }

    int keylength = strlen(argv[1]);
    int i = 0;

    for( ; i <keylength; i++)
    {
        if (!isalpha(argv[1][i]))
        {
            printf("not alpha\n");
            return 1;
        }
    }

    char *key = argv[1];
    printf("b4 %s\n", argv[1]); 

    for (int i = 0; i < keylength; i++)
    {
        if(isupper(key[i]))         
            key[i] = key[i] - 65;
        else if(islower(key[i]))    
            key[i] = key[i] - 97;
    } 

    printf("after %s\n", argv[1]);
}

Things to note.

  1. Don't try to access argv[1] until you know argc is at least 2.
  2. You need to check each char in argv[1] against isalpha in a loop. Not just pass the string pointer directly to isalpha.
  3. Also note that you are altering the command line arguments memory directly. Whilst its possible to do, its a dubious practice. It would be better to make a copy of argv[1] and alter that. I have not implemented this in my example.
Comments