Why do I have a segmentation fault?

Question

 * Description : This program allows a user to enter a password and and the name of a text file. After the user
 * has entered sufficient information, the program will encode the text and save the overwrite the file with the
 * encoded message.
 *
 */
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

char make_lower (char ch){//This converts the text of the selected file to all be lowered
int i;//initializing

    if (isalpha(ch)==1) ch=tolower(ch);//if the selected item in the file is an uppercase character, then it converts it to lowercase.
    else;//other-wise do nothing
    return ch;
}

void randomize ( char arr[], int n, int pswd )//Function that randomizes the alphabet to use while encoding
{
    int i;//initialing
    char ch;//initializing
    // Use a different seed value so that we don't get same
    // result each time we run this program
    srand (pswd);//using the seed the user has entered

    // Start from the last element and swap one by one. We don't
    // need to run for the first element that's why i > 0
    for ( i = 0; i < 26; ++i)//for loop the iterates through the alphabet array
    {
        // Pick a random index from 0 to 25
        int random = rand()%26;

        // Swap arr[i] with the element at random index
        char tmp = arr[random];//swapping
        arr[random] = arr[i];//swapping
        arr[i] = tmp;//swapping
    }}

char encode (char ch, char alpha[]){//function that encodes a message using the alphabet that was randomized
int i, tmp;//initializing

    if (isalpha(ch)== 2){//if the selected character is a lowercase then it gets swapped with the "encoded" character.
    int k = ch;
    ch = alpha[k-97];
}
return ch;}


int main(int argc,char *argv[]) {

    char alpha[26] = ("abcdefghijklmnopqrstuvwxyz");//initializing char array
    randomize(alpha, 26, argc);
    int i=0;
    char ch;
    while (argv[i] != EOF){
    ch = argv[i];
    ch = make_lower(ch);
    ch = encode(ch,alpha);
    putchar(ch);
++i;
    }
}

I don't know why I keep getting the error : Segmentation fault (core dumped). I believe I have a rogue pointer but I don't know how to fix it. I have used a debugger and still cant seem to fix the issue.

Can someone please help? Thank you!

Answer 1

The problem is that you loop on argv[] looking for EOF that you will not encounter. So your while loop will go out of range as soon as i>=argc.

 while (argv[i] != EOF){    // ouch !!! 
   ... // process argv[i]
   ++i;
 }

You have to loop on i<argc :

 for (i = 0; i<argc; i++) { 
     ... // process argv[i]
 } 

Other issues:

As a general practice, define your constant strings like this;

char alpha[] = "abcdefghijklmnopqrstuvwxyz"; //will automatically get the right length

However, the wrong length you've used (forgetting the nul-terminator of the string) is not the cause of your core dump, because in randomize() you access to this char array only with variables that are strictly less than 26, and you don't seem to use c-string functions that are depending on this terminator.

Your function encode() would also access only to the index 0 to 25 if it would be sure that encoding is done only on lower case chars. Unfortunately, here there is a risk of core dump, because isalpha(ch)==2 doesn't ensure this : isalpha() returns a non zero for alphanumerics. So you'd better use islower() instead:

if (islower(ch)) { //if the selected character is a lowercase then it gets swapped with the "encoded" character.
    int k = ch;
    ch = alpha[k-97];  // that's fine because if islower() is true k is between 97 and 122 and thus k-97 is between 0 and 25.  
}

Still other minor issues:

A similar issue is in make_lower() where you should use isupper(). But no indexing is used here, so no core dump.

Finally, I wonder if ch = argv[i]; compiles because argv[i] is a pointer to char. So you should write ch = *argv[i];

Answer 2

your alpha array has 27 elements : 26 (each letters) + '\0' (end of string terminator)

char alpha[27] = "abcdefghijklmnopqrstuvwxyz"

You should loop over argc on top of that...

for (int i=0;i<argc;++i) {
   f(argv[i]) ....
}

Note : your whole program is quite unclear :-)