please help segmentation fault ... probably simple (i'm pretty new to c)

[Branden Bobo]

I'm trying to write a Caesar cipher. but i keep getting a segmentation fault even though i'm not accessing unauthorized memory ( far as I know)

#include <stdio.h> 
#include <stdlib.h>
#include <cs50.h>
char cypherLetter(char letter, char key, bool encrepting);

int main(int argc, string argv[])
{   
    printf( "test"); // this doesn't even print
    string cypherText = "Test";
    int cypherKey = 1; //atoi(argv[1]); 
    
    for(int i = 0 ; i < argc ; i++)
    {
        
        cypherText[i] = cypherLetter(cypherText[i], cypherKey, true);
        printf("%s", cypherText);
    }
    printf("%s", cypherText);
    return 0;
}
.
.
.

[Stewbond]

This doesn't compile for me as string is not defined.
before the main, use typedef char* string, or replace string with char* everywhere.

Second, you are using argc in your counter... why is that? You aren't using argv anywhere, even if you were, would it really tell you the size of cypherText?

I think the problem though is in the way that printf is driven (this is where my breakpoint was triggered during the crash). I'm not really that used to printf as I use c++'s iostream instead so I can't give you too much info, but this is what I would do:

#include <iostream>
#include <string>
//#include <cs50.h>
char cypherLetter(char letter, char key, bool encrepting)
{
	return letter ^ key;
}

int main(int argc, char* argv[])
{   
	if (argc < 3)
	{
		std::cout << "Usage: " << argv[0] << " CypherText cypherKey" << std::endl;
		return 1;
	}

    std::string cypherText = argv[1];
    int cypherKey = atoi(argv[2]);
    
    for(int i = 0 ; i < cypherText.size() ; i++)
    {
        
        cypherText[i] = cypherLetter(cypherText[i], cypherKey, true);
        std::cout << cypherText[i];
    }
    std::cout << cypherText;
    return 0;
}

[Chervil]

This doesn't compile, as string is referenced but the header <string> is not included.
The declaration of main() is wrong.
argc is a count of how many command-line parameters were received (the first is always the name of the program itself).

My version:

#include <stdio.h>

char cypherLetter(char letter, char key, bool encrepting);

int main(int argc, char * argv[])
{
    printf( "test\n");
    char cypherText[] = "Test";
    int cypherKey = 1;

    printf("Before: %s\n", cypherText);
    for(int i = 0 ; i < strlen(cypherText) ; i++)
    {
        cypherText[i] = cypherLetter(cypherText[i], cypherKey, true);
        printf("%s\n", cypherText);
    }
    printf("After: %s\n", cypherText);
    return 0;
}

char cypherLetter(char letter, char key, bool encrepting)
{
    if (encrepting)
        return letter + key;
    else
        return letter - key;
}

[Branden Bobo]

Thanks for responding...
string is defined in <cs50>. I am a hobbiest programmer in c# and java, but I decided to try to teach myself c (c99) and c++ for better understanding. I know obviously this is a c++ forum so I apologize that my question is about c99.
but my main problem is :
I've been using printf for debug because i'm not using an IDE so I have to learn "how it works"
I seem to get the error when I call the function:

#include <stdio.h> 
#include <stdlib.h>
#include <cs50.h>
#include <string.h>
char cypherLetter(char letter, char key, bool encrepting);

int main(int argc, string argv[])
{   
    printf( "test");
    string cypherText = "Testing";
    int cypherKey = atoi(argv[1]); 
   printf("\n%s\n%d\n", cypherText,  cypherKey);
    
    for(int i = 0 ; i < strlen(cypherText) ; i++)
    {
       printf( "test1");
        cypherText[0] = cypherLetter(cypherText[0], cypherKey, true);
        printf("%s", cypherText);
    }
    printf("%s", cypherText);
    return 0;
}

char cypherLetter(char letter, char key, bool encrepting)
{
   printf( "testFunc");
    if(encrepting)
    {
        printf( "test2");
        if (letter >= 'a' && letter <= 'z')
        {
            letter -= 'a';
            letter =  ((letter + key) % 26 ) + 'a';        
        }
        else if (letter >= 'A' && letter <= 'Z' )
        {
            letter -= 'A';
            letter = ((letter + key) % 26 ) + 'A';        
        }
    
    }
    else
    {   printf( "test3");
        if (letter >= 'A' && letter <= 'Z' )
        {
            letter -= 'A';
            letter = ((letter + (26 - key)) % 26 ) + 'A';        
        }
        else if (letter >= 'a' && letter <= 'z')
        {
            letter -= 'a';
            letter =  ((letter + (26 - key)) % 26 ) + 'a';        
        }
    }   
    printf( "test4");
    return letter;
}

here my output:

jharvard@appliance (~/Dropbox/pset2): make caesar
clang -ggdb3 -O0 -std=c99 -Wall -Werror    caesar.c  -lcs50 -lm -o caesar
jharvard@appliance (~/Dropbox/pset2): ./caesar 5
test
Testing
5
Segmentation fault (core dumped)

[Chervil]

Yet again you have used an incorrect declaration for main()
int main(int argc, string argv[])
It should be like this:
int main(int argc, char * argv[])

To find the length of a c++ string, you can do one of the following:
cypherText.size() or cypherText.length();
the strlen() function is for c-strings only.

Most of the problems here are the result of mixing newer C++ strings with old C functions like printf().

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

using std::string;

char cypherLetter(char letter, char key, bool encrepting);

int main(int argc, char * argv[])
{
    printf( "test");
    string cypherText = "Testing";

    int cypherKey = atoi(argv[1]);
    printf("\nString: %s\nKey: %d\n", cypherText.c_str(),  cypherKey);

    for(int i = 0 ; i < cypherText.length() ; i++)
    {
       printf( "test1");
        cypherText[i] = cypherLetter(cypherText[i], cypherKey, true);
        printf("%s", cypherText.c_str());
    }
    printf("\nResult: %s", cypherText.c_str());
    return 0;
}

The code is actually simpler if you use c-strings (a null-terminated array of characters) rather than c++ string.

[Chervil]

I just realised I may have overlooked something. I'm thinking in (mostly) C++ terms. But if we use #define string char * then maybe the program could work virtually unchanged.

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
char cypherLetter(char letter, char key, bool encrepting);

#define string char *

int main(int argc, string argv[])
{
    if (argc < 2)
    {
        printf("Parameter missing");
        return 0;
    }
    printf( "test");
    string cypherText = "Testing";
    int cypherKey = atoi(argv[1]);
   printf("\n%s\n%d\n", cypherText,  cypherKey);

    for(int i = 0 ; i < strlen(cypherText) ; i++)
    {
       printf( "test1");
        cypherText[i] = cypherLetter(cypherText[i], cypherKey, true);
        printf("%s", cypherText);
    }
    printf("\n%s", cypherText);
    return 0;
}

[chipp]

But if we use #define string char * then maybe the program could work virtually unchanged.

why you don't simply use char * ?

[Chervil]

Because it's not my code. I'm trying to get into the same mindset as the person who wrote it.