0

My program won't write to the file after input is received. Everything else seems to work as expected.

Where have I gone wrong?

#include <stdio.h>
#include <unistd.h>

int main()
{
    char fileSelectionInput[20];

    printf("Select a file to print to: ");
    gets(fileSelectionInput);

    if (access(fileSelectionInput, F_OK ) == -1) {
        puts("It seems that this file does not exist, sorry.");
        return 0;
    }

    printf("Okay now you can type text to append\n\n");

    FILE* testFile = fopen(fileSelectionInput, "w+");

    int writesLeft = 10;

    while (writesLeft > 1)
    {
        char textInput[50];
        gets(textInput);
        fputs(textInput, testFile);
        writesLeft--;
    }

    fclose(testFile);

    return 0;
}
S.S. Anne
  • 15,171
  • 8
  • 38
  • 76
  • 1
    You should use `scanf("%19s", fileSelectionInput);` instead of `gets(fileSelectionInput);` and similarly for `textInput` but changing the size to `49`. It's safer. – S.S. Anne Mar 05 '20 at 16:21
  • 3
    You forgot to test if `fopen` returns NULL. Yes you need to do this test. Always, there is no excuse for not doing it, even if you are sure the file exists. Not testing if `fopen` return NULL is a major bug source. – Jabberwocky Mar 05 '20 at 16:22
  • 2
    @Jabberwocky Additionally, it will remove the need for the `access` test. – S.S. Anne Mar 05 '20 at 16:22
  • 5
    [Never use `gets()`](https://stackoverflow.com/q/1694036/10077)! – Fred Larson Mar 05 '20 at 16:23
  • 1
    If you want to append then you need to use `a+`. – S.S. Anne Mar 05 '20 at 16:24
  • Thank you for the comment about gets I didn't know that, should I use `scanf` or `fgets`? – Ash Ketchum Mar 05 '20 at 16:29
  • Yes I have, still the same issue. The file DOES exist as I've copied the input character for character. I also have the file open to watch for changes. I've also changed to use fopen null check and its the same issue. – Ash Ketchum Mar 05 '20 at 16:58
  • @Jabberwocky was in the process I have now done it. – Ash Ketchum Mar 05 '20 at 17:03
  • @AshKetchum Now I only see one last problem, and it's not in your code (apart from `gets` which should work fine here as long as the lengths of your input lines don't exceed the buffer site): maybe you're not looking in the right directory and the file is created correctly but it's simply not in the directory where you think it is. – Jabberwocky Mar 05 '20 at 17:08
  • @S.S.Anne Sorry, have no idea what you're referring to. – Kaz Mar 05 '20 at 21:30
  • @Kaz What do you mean by "red flags"? – S.S. Anne Mar 05 '20 at 21:31
  • @S.S.Anne https://www.merriam-webster.com/dictionary/red-flag (second entry, noun). – Kaz Mar 05 '20 at 21:32
  • @Kaz What red flags do you see? This isn't a troll post. – S.S. Anne Mar 05 '20 at 21:34

1 Answers1

2

The problem is basically the use of gets.

Try this changes bellow where I used scanf and fgets:

#include <stdio.h>
// #include <unistd.h>

int main() {
    char fileSelectionInput[20];

    printf("Select a file to print to: ");
    scanf("%19s", fileSelectionInput);  // %19s checks the size of input

    // if (access(fileSelectionInput, F_OK ) == -1) {
    //     puts("It seems that this file does not exist, sorry.");
    //     return 0;
    // }

    printf("Okay now you can type text to append\n\n");

    FILE* testFile = fopen(fileSelectionInput, "a+");

    if (testFile == NULL) {
       perror("fopen()");
       return 1;
    }

    int writesLeft = 10;

    while (writesLeft > 1) {
        char textInput[50];
        fgets(textInput, sizeof(textInput), stdin);
        fputs(textInput, testFile);
        --writesLeft;
    }

    fclose(testFile);

    return 0;
}

When you check the result of fopen, you don't have to check if the file exists with access. This makes your code more portable.

I used %19s in scanf so it won't write past the bounds of the array; 19 characters and 1 null byte fit into it.

S.S. Anne
  • 15,171
  • 8
  • 38
  • 76
TheArchitect
  • 1,160
  • 4
  • 15
  • 26
  • 1
    You're passing the wrong pointer type here, which is undefined behavior: `scanf("%s", &fileSelectionInput);`. Instead, ***don't take the address of the array***, and ***check the size***: `scanf("%19s", fileSelectionInput);` – S.S. Anne Mar 05 '20 at 17:05
  • 1
    Also, instead of `puts("Found some issue when I tried to open the file");`, use `perror("fopen()");`; it prints a more useful error message. (after the edit, you put it in the wrong block) – S.S. Anne Mar 05 '20 at 17:07
  • You can also remove the `access` to make the code more portable. It's not needed now that you check the result of `fopen`. – S.S. Anne Mar 05 '20 at 17:08
  • Thanks for the edits. I've made an edit of my own adding some notes about these things in case my comments are removed. – S.S. Anne Mar 05 '20 at 17:14