Copying one file to another: scanf keeps looping

Question

I am creating a program that takes two command-line arguments: the first line argument is the name of the file to be copied and the second is the new file. If the second argument is missing, copy the file to stdout. If both arguments are missing, the program should read from stdin and print to stdout ie. ./a.out input.txt output.txt

I did the following but I'm facing a problem where scanf keeps looping and does not quit:

int main(int argc, char *argv[]) `
{
    char text[10];
    FILE *input;
    FILE *output;
    char ch;

    printf("%s", argv[0]);
    if (argc == 3)
    {
        input = fopen(argv[1], "r");
        output = fopen(argv[2], "w");

        while ((ch = fgetc(input)) != EOF)
        {
            fputc(ch, output);
            ch = fgetc(output);
        }
    }

    if (argc == 2)
    {
        input = fopen(argv[1], "r");

        while ((ch = fgetc(input)) != EOF)
        {
            printf("%c", ch);
        }

        printf("\n");
    }

    if (argc == 1)
    {
        scanf("%c", text);
        // here it keeps looping
    }
    fclose(input);

    return 0;
}

Screen shot with cursor at end of line after some input before return is hit

You don't have a loop around the `scanf()` call. The code waits for you to enter a line of data (including the newline), and then reads the first character of that data. That's how terminals behave by default; they don't send any characters to the program reading from the terminal until you hit return (or you indicate EOF — twice if you didn't hit return yet). You've also got your `fclose(input)` misplaced and don't have an `fclose(output)` that I can see. — Jonathan Leffler, Feb 14 '20 at 02:17
See also [Canonical vs non-canonical terminal input](https://stackoverflow.com/a/358381/15168). — Jonathan Leffler, Feb 14 '20 at 02:22
Thank you! I am new to C and so, forgot that scanf waits for a line of data and yes, i also forgot to close output — , Feb 14 '20 at 03:03
You can also, do `FILE *input = argc > 1 ? fopen (argv[1], "r") : stdin, *output = argc > 2 ? fopen (argv[2], "w") : stdout;` and avoid your separate `if` conditions. Simply include a test of `if (output != stdout) fclose (output);` and the same type test for `input`. — David C. Rankin, Feb 14 '20 at 03:33

score 2 · Answer 1 · edited Feb 14 '20 at 02:29

2

It's not looping, it's waiting for input. You've requested scanf("%c"), a character, but scanf won't continue until you press enter. I think you meant scanf("%s") or getch(), but it isn't clear.

It will throw an error on the next line as fclose() is closing a FILE* that isn't initialized in the case of only 1 argument.

You could probably rework this homework example to use fopen() on STDIN/STDOUT in the case of missing parameters so that you aren't writing redundant code.

edited Feb 14 '20 at 02:29

David C. Rankin

81,885
6
58
85

answered Feb 14 '20 at 02:24

Chris_P

88
5

2

Always worth mentioning if you mention `getch()` that it is 100% non-portable to anything other than DOS/windows. – David C. Rankin Feb 14 '20 at 02:29
3

Using `scanf("%c", text)` where you have `char text[10];` is safe and 'correct', though there's ten times as much storage as necessary. Using `scanf("%s", text)` would also work as long as no more than 9 characters were typed after white space and before the next white space. Using `scanf("%9s", text)` would be safe even if you typed 'supercalifragilisticexpialidocious' instead of 'ice-cream' or 'z'. – Jonathan Leffler Feb 14 '20 at 02:32

score 1 · Answer 2 · answered Feb 14 '20 at 08:06

There are multiple problems in your code:

You output argv[0] to stdout: if argc < 3, you are supposed to copy to stdout, this extra output is corrupting the output. You might instead output to stderr or remove this line completely.
you do not check for fopen failure, causing undefined behavior if either file cannot be opened.
ch has type char which is too small to accommodate for all return values of fgetc(). On machines with 8-bit bytes, fgetc() has 257 possible return values, you must make ch an int to reliably distinguish EOF from all other return values.
in the fgetc() / fputc() loop, you read 2 bytes in each iteration of the loop but only write one byte.
the scanf() on the last case is simply waiting for input as you are supposed to copy from stdin to stdout, but the program will stop as soon as you hit the Enter key. You should just use the same fgetc()/fputc() loop for all copying loops.
you should include <stdio.h>

Here is a modified version:

int main(int argc, char *argv[]) {
    FILE *input = stdin;
    FILE *output = stdout;
    int ch;

    if (argc > 1) {
        input = fopen(argv[1], "r");
        if (input == NULL) {
            fprintf("cannot open input file %s\n", argv[1]);
            return 1;
        }
    }
    if (argc > 2) {
        output = fopen(argv[2], "w");
        if (output == NULL) {
            fprintf("cannot open output file %s\n", argv[2]);
            return 1;
        }
    }
    while ((ch = fgetc(input)) != EOF) {
        fputc(ch, output);
    }
    if (argc > 2) {
        fclose(output);
    }
    if (argc > 1) {
        fclose(input);
    }
    return 0;
}

Copying one file to another: scanf keeps looping

2 Answers2