pset 2 segmentation fault?

Question

I have segmentation fault somewhere, with gdb i found that it gives the fault at return a inside cypher() function, can i fix it?:

/*
    caesar.c

    Carlos Lopez J.
    luchillo17@gmail.com

    This program encrypts a text in Caesar cypher.
*/

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

char cypher(char c, int k);
char toAscii(int n);

int main ( int argc, string argv[])
{
    if (argc !=2){
        printf("Hell no, you need only one comand line argument.\n");
    return 1;
    }

    int k = atoi(argv[1]);

    string s1="Be sure to drink your Ovaltine!";

    for(int i = 0, n = strlen(s1); i < n; ++i){
        s1[i]=cypher(s1[i] , k);
    }
    printf("Text: %s\n", s1);

    printf("Distancia wrap: %c\n", 'A'+25);
    return 0;
}

char cypher(char c, int k){
    char a;
    if(isupper(c)){
        a=(((c - 'A')+k)%26)+'A';
        printf("%c",(((c - 'A')+k)%26)+'A');
    } else if(islower(c)){
        a=(((c - 'a')+k)%26)+'a';
        printf("%c",(((c - 'a')+k)%26)+'a');
    } else {
        a=c;
        printf("%c",c);
    }
    return a;
}

char toAscii(int n){
    char s[('z'-'a')+('Z'-'A')+2];
    for(int i = 0, tam = ('z'-'a')+('Z'-'A')+2; i < tam; ++i){
        if (i < 'Z'-'A'+1){
            s[i]='A'+i;
        } else {
            s[i]='a'+i-tam/2;
        }
    }
    printf("ToAscii: %c\n", s[n]);
    return s[n];
}

Answer 1

Why are you casting? Run this and see what happens:

#include <stdio.h>
int main(void) 
{
    int a = 97;
    printf("a is an int: %i but a is also a char: %c\n", a, a);

    char b = 'b';
    printf("b is a char: %c but is also an int: %i\n", b, b);
}

---

edit

I think you are complicating the problem. In C, a char is an int so you don't need to convert anything.

Try this:

printf(" 'a' + 3 = %c\n", 'a' + 3);  
printf(" 'a' + 3 = %d\n", 'a' + 3);

You will get:

'a' + 3 = 'd'
'a' + 3 = 100

I didn't need to convert the value of 'a' to an int first. The C language is different from many other languages that treat chars as separate from ints.

---

edit again

If you take the formula that you are using in your for loop above, and simply print the characters rather than saving them to a new char array, it should certainly work.

if(isupper(s[i]))
    printf("%c", (((s[i] - 'A')+k)%26)+'A');

etc.

I think you are needlessly complicating things by creating new strings to hold the changed values. At this stage in the course, you haven't been taught enough about strings and char arrays, etc., to be expected to do a lot of manipulation. The simple answer is to print each char as you go, or, change the existing char and then print it.

if(isupper(s[i]))
    s[i] = (((s[i] - 'A')+k)%26)+'A');

...

printf(%s, s);

There is no need to create any new string because you don't need to keep the existing one.

Answer 2

Ok the issue in my code is that i'm trying to modify s1 in the same line of the cypher of s1 call, that gives a segmentation fault, to correct that i need other variable, but as i need a string of the same size of s1, say s2 need to be a new string, but as a string is just a typedef for char* string[], we need to allocate the memory needed for the string like:

s2=malloc((strlen(s1)+1)*sizeof(char));
s2[strlen(s2)]='\0';

Where +1 is for the '\0' that means end of string, the other things can be looked up in the malloc documentation, then the second line is for ensure that s2 have the '\0' at the end of it, i don't know if the second line is necessary, then:

for(int i = 0, n = strlen(s1); i < n; ++i){
    s2[i]=cypher(s1[i], k);
}

This for cyphers s1 character by character and return it to s2 in the correct index, or in the coresponding place in s2, then you can print s2 as the cyphered data as a string:

printf("Cypher: %s\n, End: %i\n", s2, s2[strlen(s2)]);

Feel free to ask me if you don't understand.