0

So the pseudo code is:

FIle existing = Read whole content of file.

String newcontent = "Blah Blah This is first line" + existing

f.write(newcontent);

The actual code is

void Prepend(char *string, char *file_name)
{   

    char buf[100];
    FILE *fp1, *fp2;
    Append(string, "temp_file");
    
    fp1 = fopen(file_name, "a+");
    fp2 = fopen("temp_file", "a+");
    
    if (!fp1 && !fp2) {
        printf("Unable to open/"
               "detect file(s)\n");
        exit(1);
    }
 
    fprintf(fp2, "\n");
 

    while (!feof(fp1)) {
        fgets(buf, sizeof(buf), fp1);
    }
 
    rewind(fp2);
 

    while (!feof(fp2)) {
        fgets(buf, sizeof(buf), fp2);
    }
      fclose(fp1);
      fclose(fp2);
      remove(temp_file);
}

/*-----------------------------------------------------------------------------*/

void Append(char *string, char *file_name)

{   

    FILE *file = fopen(file_name, "a");
    
    if(file==NULL)
    {
        printf("Error opening file.");
    }
    else 
    {
    
        fputs(string, file);    
        
    }
    
    fclose(file);
}

I know there is a way with fseek maybe and other pointer related stuff, but I thought it would be better and faster for me to go with the "easy way" and just make a new temp file and delete it right afterwards.

Well, it also doesn't work.

NoobCoder
  • 513
  • 3
  • 18
  • Note that `if (!fp1 && !fp2)` is only true if both files can't be opened. Use `if (fp1 == NULL || fp2 == NULL)` (or `!fp1` etc if you prefer — I don't, but I'm probably in a minority) to make the test true if either (or both) files can't be opened. – Jonathan Leffler Mar 01 '21 at 04:15
  • If you open files in append mode, all writing will occur at the end of the file (and the file won't be truncated). Only if the file is empty or non-existent will data be written at the start. – Jonathan Leffler Mar 01 '21 at 04:18
  • @JonathanLeffler Yes I know. But that's the purpose here. I'm creating a non-existing file called `temp_file` and writing into him the input string from the user. Then, i'm appending the contents of the original file to this string, and that's how I "prepended" the user string before the original data. – NoobCoder Mar 01 '21 at 04:27
  • No; you're opening an existing file `temp_file` or creating a file if it doesn't exist. You don't know that the file doesn't exist; you are just hoping it doesn't exist. You could use `"w"` or `"w+"` mode to create an empty file or truncate an existing file. Now, you are probably correct that `temp_file` doesn't exist if everything works OK. – Jonathan Leffler Mar 01 '21 at 04:29
  • 1
    Note that [`while (!feof(file))` is always wrong](https://stackoverflow.com/q/5431941/15168). – Jonathan Leffler Mar 01 '21 at 04:30
  • It's not particularly obvious why you read all the way through `fp1` when you do nothing with the data that's read. After you've printed the newline to `fp2`, rewinding it doesn't make any difference to where the next data is written because the file is open in append mode. All writing will occur at the end of the file. That's what append mode means. – Jonathan Leffler Mar 01 '21 at 04:32
  • Note that `if (file == NULL) { … } else { … } fclose(file);` has the `fclose()` in the wrong place. If the file fails to open, then `fclose()` is likely to crash. That call should be in the `else` block. You should report error messages on `stderr`, not `stdout`; you should include a newline at the end of the message; you should include the file name in the error message. – Jonathan Leffler Mar 01 '21 at 04:34

2 Answers2

1

Here a little example, using only two instead of three files.

void prepend(const char *filename, const char *str)
{
    // open the orig file (read only)
    FILE *orig = fopen(filename, "r");

    // open tmp file (read / write)
    FILE *tmp = fopen("temp_file", "w+");

    // write string into tmp file (prepend)
    fwrite(str, 1, strlen(str), tmp);

    // file size of orig
    fseek(orig, 0L, SEEK_END);
    long orig_size = ftell(orig);
    rewind(orig); // set pos to 0

    // transfer bytes from orig to tmp (append)
    char *buf = malloc(orig_size);
    fread(buf, 1, orig_size, orig);
    fwrite(buf, 1, orig_size, tmp);
    free(buf);

    // close orig and delete it
    fclose(orig);
    remove(filename);

    // close and rename tmp file as orig
    fclose(tmp);
    rename("temp_file", filename);
}

Note: To keep things short, no error checking involved.

Erdal Küçük
  • 4,810
  • 1
  • 6
  • 11
1

since you're using a temporary file, a secondary buffer should not be required, just append the text, then move the old file contents into the new one.

A few things you should lookout for,

  • if (!fp1 && !fp2) is only true if both files failed to be opened ,its better to check the results individually so we know where we failed. You could have replaced && with || but it will be ambiguous why it failed unless you re-check the file pointer values.
  • It seems you were never writing the data from fp1 into fp2 I only see calls to fgets(), you read fp1 into buf then you read fp2 into buf, doing nothing with the data both times. If the files are larger than 100 bytes you'll end up with the last 100 or less bytes of data being stored in buf.
  • It's not necessarily the worst thing, but it is good practice to only open files with the permissions you require. opening a file you only need to read from in 'a+' mode is kind of odd
  • before creating new files its a good idea to check that a file doesn't already exist by that name. You might be better off to create the file in the system's temporary directory or a dedicated tmp dir for your program. Using a unique string for it's name is also a good idea. perhaps something like "[my prog]-[original file name]-[.tmp]" instead of such a general name.

A short example of a method that should work, with error checking (not necessarily the best error checking but gives you an idea where errors can occur):

#include <sys/types.h>
#include <sys/stat.h>
#include <errno.h>
#include <string.h>

int Prepend(char *string, char *file_name)
{
    FILE *fp1 = NULL, *fp2 = NULL;
    struct stat fs;
    if (stat("temp_file", &fs) == 0) {
        printf("Error, there is already a file named \"temp_file\" in the current working directory\n");
        return -1;
    }
    if ((fp1 = fopen(file_name, "r")) == NULL) {
        printf("Error : %s\n", strerror(errno));
        printf("Failed to Open file %s\n", file_name);
        return -1;
    }
    if ((fp2 = fopen("temp_file", "w")) == NULL {
        printf("Error : %s\n", strerror(errno));
        printf("Unable to create Temporary file\n");
        fclose(fp1);
        return -1;
    }
    if (fputs("Prepended String\n", fp2) == EOF) {
        printf("Failed to write string to file\n");
        fclose(fp1);
        fclose(fp2);
        if (remove("temp_file") != 0)
            printf("Error : %s, while removing temp_file\n", strerror(errno));
        return -1;
    }
    int c;
    while((c = fgetc(fp1)) != EOF) {
        if(fputc(c, fp2) == EOF) {
            printf("Error while transfering data from %s to %s\n", file_name, "temp_file");
            fclose(fp1);
            fclose(fp2);
            if (remove("temp_file") != 0)
                printf("Error : %s, while removing temp_file\n", strerror(errno));

            return -1;
        }
    }
    fclose(fp1);
    if (remove(file_name) != 0) {
        printf("Error : %s, while removing %s\n", strerror(errno), file_name);
        fclose(fp2);
        return -1;
    }


    fclose(fp2);
    if (rename("temp_file", file_name) != 0) {
        printf("Error : %s, while renaming temp_file\n", strerror(errno));
        return -1;
    }
    return 0;
}

fclose() can also return EOF on error and errno is set to indicate the error. However, any further access (including another call to fclose()) to the stream results in undefined behavior. so you can print the error but there isn't much you can do about it.

pseudovella
  • 209
  • 1
  • 8
  • Thank you. It works. What exactly is different between your code and mine (other than the obvious)? I mean, from your point of view, where did I go wrong ? – NoobCoder Mar 01 '21 at 08:41
  • @NoobCoder, the first big problem the program hits would have been the `if(!feof())` bit. I'm assuming the temporary file would get created and your string was written but then after that nothing else happens right? It's good practice to post the outcome of your project, rather than just say "it doesn't work". – pseudovella Mar 01 '21 at 17:15
  • You are right, and I'll do it in the next time. What about checking after `fclose`? Shouldn't I do that? Because `fclose` can fail. and after `remove`? and to `unlink` after a `remove` to make sure there are not soft links anywhere? – NoobCoder Mar 01 '21 at 17:30
  • Sorry on my phone so its hard to read your code and comment at the same time... But basically `while(!feof())` is actually technically right, you aren't doing anything with the data. That loop will keep going and overwriting the contents of buf until the file is read, and then nothing is written. – pseudovella Mar 01 '21 at 17:46
  • then you go on to do the same thing again to fp2, if you were to put both reads and writes in the one while loop it could work I suppose. And yes any of these functions could technically fail and set `errno` so you can check what went wrong or pass it on as your return value. – pseudovella Mar 01 '21 at 17:50
  • you can safely ignore `fclose()` errors IMO as the stream will no longer be associated with the file or its buffers. But it would be wise to check the calls to remove and rename. – pseudovella Mar 01 '21 at 18:06