1

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
    Are you seriously trying to allocate an array of `INT_MAX` bytes *on the stack*? – EOF Jun 10 '15 at 22:41
  • 1
    `char filename1[INT_MAX]` --> `char filename1[FILENAME_MAX]` – BLUEPIXY Jun 10 '15 at 22:43
  • As a followup to to @EOF, I would be surprised if that even ran. There are better ways to make things safe from buffer overruns than blowing up the stack yourself with those arrays, – Michael Dorgan Jun 10 '15 at 22:43

2 Answers2

4

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;
}
Community
  • 1
  • 1
rost0031
  • 1,894
  • 12
  • 21
1

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);
Alexandre Severino
  • 1,563
  • 1
  • 16
  • 38
  • 2
    It is more likely that the OP does NOT need to allocate `2147483647` characters. Second -- **do NOT cast the return of `malloc`**. – David C. Rankin Jun 10 '15 at 22:54
  • @DavidC.Rankin Well. Casting the `malloc` allows for better compatibility with `C++`. If you are using pure C and not a **C++ compiler** then I completely agree with you. I'll edit the answer, nevertheless. But I warn the OP that the `malloc` won't work on **C++ compilers** (at least not **MSDN**) without a cast. – Alexandre Severino Jun 10 '15 at 23:00
  • @AlexandreSeverino: casting malloc in C may result in the compiler hiding a warning which could cause crashes: http://stackoverflow.com/a/605858/951784 – rost0031 Jun 10 '15 at 23:02
  • 1
    Agreed, but the OP should be also warned that: "True. However, in C++ the cast is required, so if you want your code to work in both, you'll have to compromise. But in pure C, don't do the cast for the reasons you stated. – jalf Mar 3 '09 at 10:36" – Alexandre Severino Jun 10 '15 at 23:05
  • The point being -- do you see a `C++` tag attached to the question? I understand what you are saying about `C++`, but it doesn't belong in answer to a `C` tagged question. You edit is fine, but may still be a bit bewildering to others. – David C. Rankin Jun 10 '15 at 23:14
  • 1
    I do that mostly because new programmers will most likely use popular compilers (which are all C++) and won't get why the code does not compile. But, if you guys think that my edit do not attend the community constraints for tags, there we go with another edit. – Alexandre Severino Jun 10 '15 at 23:16