Simple function code to delete a line from a text file by making a temporary text file that will store the new content once the line has been deleted and replacing the old Storage.txt file with the temporary file.

The delete() function works but my only problem seems to be the rename() function that seemingly won't do as intended.

THE CODE

void delete() {
    struct task task;
    FILE *fp;
    char str[100];
    char ch;
    int delete_line = 0;
    fp = fopen("Storage.txt", "r");
    if (fp == NULL) {
        printf("Error opening file");
        fopen("Storage.txt", "w");
        exit (1);
    }
    printf("\n\n\nAll Tasks\n");
    printf("----------\n\n");
    do {
        ch = fgetc(fp);
        printf("%c", ch);
    } while (ch != EOF);
    fclose(fp);
    int line_no,ret;
    char filename[] = "Storage.txt";
    char newname[] = "temp.txt";
    FILE *file, *temp;
    file = fopen("Storage.txt", "r");
    temp = fopen("temp.txt", "w");
    printf("Select Line to delete: ");
    scanf("d", &delete_line);
    getchar();
    temp = fopen("temp.txt", "w");
    while (fgets(str, 99, fp) != NULL) {
        line_no  ;
        if (line_no != delete_line) {
            fputs(str, temp);
        }
    }
    fclose(file);
    fclose(temp);
    remove(filename);
    ret = rename(newname, filename);
    if (ret == 0) {
        printf("File renamed successfully");
    } else {
        printf("Error: unable to rename the file");
    }
}

CodePudding user response：

There are some problems in the code:

• ch must be defined with type int to detect EOF reliably.

• the do/while loop to read the file contents outputs the EOF indicator before testing it. You should use while ((ch = fgetc(fp)) != EOF) putchar(ch);

• the identifier delete should be avoided to avoid confusing C programmers, use delete_line instead.

• you should test for failure of fopen and remove and display the cause of the error.

• if opening the file for reading fails, why do you create the file with fopen("Storage.txt", "w") ?

• file temp.txt is open twice, which may prevent the rename operation on legacy systems.

• line_no is not initialized. It should be initialized to 1 if lines are numbered starting at 1.

• reading lines into an array is not reliable for this task as lines longer than 99 bytes will be counted more than once.

Here is a modified version:

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

void delete_line() {
    const char *filename = "Storage.txt";
    const char *tempname = "temp.txt";
    int ch;

    FILE *fp = fopen(filename, "r");
    if (fp == NULL) {
        fprintf(stderr, "Cannot open file %s: %s\n", filename, strerror(errno));
        exit(1);
    }
    printf("\n\n\nAll Tasks\n");
    printf("----------\n\n");
    while ((ch = getc(fp)) != EOF)  {
        putchar(ch);
    }
    fclose(fp);

    int delete_line = 0;
    printf("Select Line to delete: ");
    if (scanf("d", &delete_line) != 1) {
        fprintf(stderr, "invalid or missing input\n");
        exit(1);
    }
    // read and discard the rest of the user input line
    while ((ch = getchar()) != EOF && c != '\n')
        continue;

    FILE *file = fopen(filename, "r");
    if (file == NULL) {
        fprintf(stderr, "Cannot open file %s: %s\n", filename, strerror(errno));
        exit(1);
    }
    FILE *temp = fopen(tempname, "w");
    if (temp == NULL) {
        fprintf(stderr, "Cannot open file %s: %s\n", tempname, strerror(errno));
        fclose(file);
        exit(1);
    }
    int line_no = 1;
    while ((ch = getc(file)) != EOF) {
        if (line_no != delete_line)
            putc(ch, temp);
        if (ch == '\n')
            line_no  ;
    }
    fclose(file);
    fclose(temp);

    if (remove(filename)) {
        fprintf(stderr, "Cannot remove %s: %s\n", filename, strerror(errno));
    } else {
        if (rename(tempname, filename)) {
            fprintf(stderr, "Cannot rename %s as %s: %s\n",
                    tempname, filename, strerror(errno));
        }
    }
}

CodePudding user response：

Your code opens the "temp.txt" file twice:

temp = fopen("temp.txt", "w");
...
temp = fopen("temp.txt", "w");

And closes it once. That will leave one open file descriptor to the file, untill the program exits.

remove() uses unlink() for deleting files. The man page of unlink() says:

If the name was the last link to a file but any processes still have the file open the file will remain in existence until the last file descriptor referring to it is closed.

Ensure that all file descriptors are closed when not needed anymore.

The rename may fail, if file of oldpath or newpath is still open.

CodePudding user response：

temp = fopen("temp.txt", "w"); Call it twice

CodePudding user response：

the two main bugs here are:

1. scanf(d, ...) instead of scanf(%d, ...)

scanf() needs a format string to know how to parse the input, just like printf() (the f is for format) needs it to know how to construct the output; and their format string syntax is almost the same.

2. Unintialized line_no, meaning that it's not guaranteed to start at 0/1, thus it might not ever be equal to delete_line, and will not delete the line.