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


    Carlos Lopez J.

    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;
        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 {
    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){
        } else {
    printf("ToAscii: %c\n", s[n]);
    return s[n];

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);


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.

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


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.

    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.

  • I did that inside a loop like a being the var i and worked, but my code returns a int that is equivalent to 'O' in ascii and do the same as you want me to do and didn't work, don't know why? – Carlos Esteban Lopez Jaramillo Nov 22 '14 at 16:25
  • if you look at my function "toAscii(int n)" i do that, converts n in an alphabetic character, but it doesn't print anithing even if the int is the "O" ascii equivalent, i tried all and didn't work, also tried your suggestion and didn't work. – Carlos Esteban Lopez Jaramillo Nov 22 '14 at 18:56
  • see my edited answer. – curiouskiwi Nov 22 '14 at 20:24
  • You don't get me, i don't convert the char to an int, i just rest it 'A', then sum variable k, then use module '%26' and sum 'A' again and that was the arg of printf with %c in there, and it didn't work, it keeps printing 0 or "", that was the original algorithm but as it didn't work i tried other things. – Carlos Esteban Lopez Jaramillo Nov 22 '14 at 22:21
  • see my edit again. – curiouskiwi Nov 22 '14 at 22:52

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:


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.

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .