Segmentation Fault when trying to increment letters

Question

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

int main(int argc, string argv[])
{
    if(argc > 2){ printf("too many arguments\n"); return 51; }
    if(argc < 2){ printf("too few arguments\n"); return 50; }
    if(strlen(argv[1]) > 4){ printf("Password is greater than 4 characters\n"); return 52; }

    if(argc == 2) //make sure there are enough args
    {
        string hash_guess = "A";

        while(crypt(hash_guess, "50") != argv[1]) //while answer not correct
        {
            while(hash_guess[0] <= 'Z' && hash_guess[0] >= 'A')
            {
                hash_guess[0] = hash_guess[0] + 1;
                printf("%s", hash_guess);
            }
        }

    }

}

I am trying to increment letter by letter through a word, so it would go a then b then c until z then it will go aa, ab, ac, ad.... then ba bb bc bd... then za... zz... zzz. I'm starting with one letter, but I get error "segmentation fault"

There is no C type called `string`. Use `char hash_guess[] = "A"` — ryyker, Jul 05 '17 at 18:29
Id' help if we had the crypt() function. Also, `string` is not a C type. Is this C or C++? If so, what is `string`? — Michael Dorgan, Jul 05 '17 at 18:29
Ignoring the rest of the code why does something like this make a segmentation fault: string[x]++; — sruly, Jul 05 '17 at 18:35
@KevinDTimm - Regarding your edit, I did not want to revert before checking with you first,... `cs50` falls within the bounds of `C`, so I believe the C tag can still apply. i.e., stated in the tag's description: _...This tag should be used for questions which use the cs50.h header or cs50 library along with the C tag._ — ryyker, Jul 05 '17 at 18:38
Depending on what type `string` is, that increment could be undefined behavior. Imagine if it were in ROM and you told it to write back an incremented address. — Michael Dorgan, Jul 05 '17 at 18:44
@ryker - IMNSHO, it's not `c` when it uses the `cs50` string type - so many troubles come from that one particular `enhancement` (as you found in your answer) — KevinDTimm, Jul 05 '17 at 21:13
Since you are studying through the horrible cs50 tutorial, you will learn incorrect use of C strings from the start. This is the source of your bug. Throw cs50 out the window and get a good book instead. — Lundin, Jul 06 '17 at 12:52

ryyker · Accepted Answer · 2017-07-06T12:52:24.583

0

I am surprised you are getting a seg-fault. The code you are showing should not even compile.

The line:

   string hash_guess = "A";

uses string as a type. Unless you have typedef'd string in a part of your code somewhere not shown, it cannot be used to create a variable.

Try:

   char hash_guess[] = "A";//creates array of char, 2 bytes long:  |A|0|

Even then, if you need this variable to contain more than just 2 bytes, create it with more space:

   char hash_guess[10] = "A"; Creates space for 10 bytes: |A|0|0|0|0|0|0|0|0|0|

Remember that a C string is defined as an array of char terminated with a NULL.

EDIT Per @MichaelDorgan's finding, cs50 typedefs String as char *. So the offending line now becomes equivalent to:

   char *hash_guess = "A";

Which is why it compiles and runs. The segfault could be happening when you explicitly attempt to write to a location in memory that you do not own, which is UB. (however, it is not clear from looking at your code that this is what is happening, just a guess)

Another conventional way to create and initialize a pointer, then create space (for more than two bytes):

String hash_guess = {0};
hash_guess = calloc(10, 1);
if(hash_guess)
{
    //... (your code here)
    //hash_guess can now contain up to 9 characters and the NULL terminator.
}
free(hash_guess);

edited Jul 06 '17 at 12:52

answered Jul 05 '17 at 18:42

ryyker

22,849
3
43
87

The problem is, we just don't know what `string` is. Is it a naked pointer? Perhaps const value to a ROM table? Or maybe even just a `char` that his special system knows to auto terminate. This falls under need more info I believe. – Michael Dorgan Jul 05 '17 at 18:49
1

Yup, that's what I needed to do. a char array works. – sruly Jul 05 '17 at 18:49
@MichaelDorgan - At the very least we know that within the bounds of what is shown, `string` should not be able to create a variable as it is not a type. – ryyker Jul 05 '17 at 18:50
1

Looked it up - string in cs50 is typedef to char*. So now we know. – Michael Dorgan Jul 05 '17 at 18:51
@MichaelDorgan - good find. That changes things a little. It is not evident in the shown code, but if at any point more than two bytes are assigned to `hash_guess`, then a `seg-fault` becomes a possibility. – ryyker Jul 05 '17 at 19:01
cs50 is an crap library that defines `string` as `char*`. Using this crap library/tutorial is the source of the bug. – Lundin Jul 06 '17 at 12:53

score 0 · Answer 2 · answered Jul 05 '17 at 19:02

Ok the reason for the segfault, just for information, is that you are using compiler's read only memory as temporary storage.

typedef string char*;

string hash_guess = "A";

The compiler generates a 2 entry constant memory array of 'A' and '\0'. You cannot write to this memory. It is Undefined Behavior. (UB)

        while(hash_guess[0] <= 'Z' && hash_guess[0] >= 'A')
        {
            hash_guess[0] = hash_guess[0] + 1;

...

Is illegal. You need a temp char array to hold your guess for this to be legal.

char my_guess[5];  // You specify a max length of 4.  Make a constant soon please.
strcpy(my_guess, hash_guess);  // Real code never uses this function for safety reasons.

// Beware that later when going to multi-letter check, you will 
// need to re- NUL terminate the string...

        while(my_guess[0] <= 'Z' && my_guess[0] >= 'A')
        {
            my_guess[0] = my_guess[0] + 1;
// ...

my_guessis on the stack and safely usable.

Segmentation Fault when trying to increment letters

2 Answers2