Mister Tusk
Mister Tusk

Reputation: 325

Use write() to write char* to file from user input

I'm trying to write a function that takes in a path from stdin and saves that path to a file. Despite many attempts, I have lost any kind of sense of how to properly do it. Can someone show me how to do it right? This is my code:

void char_to_file(const char *pathname, const char *dest)
{
        int fd_1;
        if (fd_1 = open(dest, O_WRONLY | O_CREAT | O_TRUNC, 0666) == -1)
                custom_error_shout("OPEN FD_1");
        while (*(pathname) != '\0')
        {
                *(pathname++);
                if (write(fd_1, &pathname, 1) == -1)
                        custom_error_shout("WRITE TO FILE");
        }

        if (close(fd_1) == -1)
                custom_error_shout("CLOSE FD_1");
}

The file will be created but nothing will be written in it. No errors have come out of this.

Upvotes: 3

Views: 144

Answers (2)

David C. Rankin
David C. Rankin

Reputation: 84561

The reason nothing is written is you reset fd_1 to the result of the conditional open(..) == -1, which unless open fails means the result will be 0 (STDIN_FILENO). If the open() does fail and returns -1, then fd_1 will equal 1 (STDOUT_FILENO). So you are trying to write to stdin unless open() fails.

This is due to your failure to wrap your assignment in parentheses, e.g. if (fd_1 = open() == -1) -- which should be if ((fd_1 = open()) == -1). Otherwise == (relational operator) has higher operator precedence than = (simple assignment) and the result of open(..) == -1 is assigned to fd_1.

To correct the problem, you need:

if ((fd_1 = open(dest, O_WRONLY | O_CREAT | O_TRUNC, 0666)) == -1)

(note: your choice of mode of 0666 will be subject to the system umask and will likely results in actual permissions of 0644. See man 2 umask)

Your write command should pass the address of the first character, not a pointer to that address, e.g.

    if (write(fd_1, pathname, 1) == -1)

You should enable compiler warnings. E.g. -Wall -Wextra -pedantic for gcc/clang or /W3 for VS (for other compilers, check the options). With warnings enable, you should receive the warning about a computed value not used with:

    *(pathname++);

To advance the pointer, simply use pathname++;. Dereferencing the result has no purpose if it is not being assigned or otherwise being used.

If you have no other issues with the strings in pathname or dest, and your error macros work properly -- then this should fix your problem. (with the note, using fopen and file-stream operations like fputs() makes much more sense).

Let me know if you have further questions.

Upvotes: 1

Carey Gregory
Carey Gregory

Reputation: 6846

You sure chose the hard way to do this. Like @tadman suggested in comments, try this instead:

void char_to_file(const char *pathname, const char *dest)
{
    FILE *fp;
    fp = fopen(dest, "w");
    if (fp == NULL)
    {
        custom_error_shout("Something went wrong opening the file");
        return;
    }

    if (fputs(pathname, fp) == EOF)
        custom_error_shout("Something went wrong writing to the file");

    fclose(fp);
}

Upvotes: 1

Related Questions