What's wrong with my decoder? Why does it freeze after prompting the user?

112 views Asked by At

The goal is to prompt the user to either encode an input file and write the code to an output file or decode an input file and write the message to an output file. Whenever I run the program, although it compiles, it stops responding as soon as the prompt character is entered.

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

int encode(FILE * fp1, FILE * fp2);
int decode(FILE * fp1, FILE * fp2);
int main() {
       char prompt;
       printf("Would you like to encode a message into a file?\n");
       printf("Or would you like to decode a message from a file?\n");
       printf("Enter e to encode or d to decode.\n");
       scanf("%c", prompt);
       FILE *fp1, *fp2;
       char filename1[INT_MAX], filename2[INT_MAX];
       printf("Enter input file: ");
       scanf("%s", filename1);
       fp1 = fopen(filename1, "r");
       printf("Enter output file: ");
       scanf("%s", filename2);
       fp2 = fopen(filename2, "w");
       if (fp1 == NULL) {
               printf("The file does not exist.\n");
               return 0;
       }
       if (fp2 == NULL) {
               printf("The file oes not exist.\n");
               return 0;
       }
       switch (prompt) {
              case 'e':
                   encode(fp1, fp2);
                   break;
              case 'E':
                   encode(fp1, fp2);
                   break;
              case 'd':
                   decode(fp1, fp2);
                   break;
              case 'D':
                   decode(fp1, fp2);
                   break;
              default:
                      break;
       }
       return 0;
    }

int encode(FILE * fp1, FILE * fp2) {
     char ci, co;
     while (fscanf(fp1, "%c", ci) != EOF) {
           for (ci = 97; ci <= 122; ci++)
               co = ci - 64;
           for (ci = 48; ci <= 57; ci++)
               co = ci + 11;
           switch (ci) {
                case 32:
                     co = 69;
                     break;
                case 10:
                     co = 70;
                     break;
                case 13:
                     co = 71;
                     break;
                default:
                     break;
           }
           fprintf(fp2, "%c", co);
     } 
     printf("Your message has been encoded.\n");
     system("pause");
     return 0;
} 

int decode(FILE * fp1, FILE * fp2) {
     char ci, co;
     while (fscanf(fp1, "%c", ci) != EOF) {
           for (ci = 33; ci <= 58; ci++)
               co = ci + 64;
           for (ci = 59; ci <= 68; ci++)
               co = ci - 11;
           switch(ci) {
                case 69:
                     co = 32;
                     break;
                case 70:
                     co = 10;
                     break;
                case 71:
                     co = 13;
                     break;
                default:
                     break;
           }
           fprintf(fp2, "%c", co);
     } 
     printf("Your message has been decoded.\n");
     system("pause");
     return 0;
}
2

There are 2 answers

3
rost0031 On

There may be other problems but the first one is here:

scanf("%c", prompt);

should be

scanf("%c", &prompt);

See full function definition and example here

Also,

char filename1[INT_MAX], filename2[INT_MAX];

There's no way that this is a good idea. Either allocate dynamically or at least use a more reasonable number for a filename. 256 should be plenty for a filename. If you're really worried about a buffer overflow in your homework assignment, you can use a technique mentioned here to properly allocate memory for an input string (as long as you're using a GNU compiler):

char *str;
printf("Enter your filename:\n");
scanf("%ms", &str);

printf("Hello %s!\n", str);
free(str);

The %m will automatically allocate a string of proper size, avoiding buffer overflows.

This one is just for readability:

    switch (prompt) {
    case 'e':
        encode(fp1, fp2);
        break;
    case 'E':
        encode(fp1, fp2);
        break;
    case 'd':
        decode(fp1, fp2);
        break;
    case 'D':
        decode(fp1, fp2);
        break;
    default:
        break;
}

Can be condensed down to:

switch (prompt) {
    case 'e':
    case 'E':
        encode(fp1, fp2);
        break;
    case 'd':
    case 'D':
        decode(fp1, fp2);
        break;
    default:
        break;
}
6
Alexandre Severino On

First:

scanf should have a reference to a char address for its second parameter. You are passing its value instead. rost0031 mentions that on his answer.

Second:

you'll very likely receive a Stack Overflow from instantiating big arrays of char on the stack. So do like this instead:

char* filename1 = malloc(INT_MAX);
char *filename2 = malloc(INT_MAX);

In case you don't know what a malloc does, it basically dynamically allocates memory on the Heap (which is much more resourceful than the Stack). These kind of allocation require that you free the memory after you are finished using them (otherwise you'll get memory leaks):

free(filename1);
free(filename2);

Third:

Your decode and encode functions do not need to return int, since you are not returning anything useful and your calls do not expect a result. You can make them void and remove the return 0; before the end of the scope:

void decode(FILE * fp1, FILE * fp2);
void encode(FILE * fp1, FILE * fp2);