1

I cannot find the problem in my code. readFile function works well, but writeFile function does not make any changes in the file:

#include <iostream>
#include <fstream>

using namespace std;
const int BUF_SIZE = 1024;

void readFile(fstream &file, char buffer[BUF_SIZE]);
void writeFile(fstream &file);

void readFile(fstream &file, char buffer[BUF_SIZE])
{
    int position;
    cout << "Please enter a position to read from the file some info" << endl;
    cin >> position;
    file.seekg(position, ios::beg);
    file.read((char*) buffer, BUF_SIZE); // <<<<<

    for(int i = 0; i < file.gcount(); i++){
        cout << buffer[i];
    }
}

void writeFile(fstream &file)
{
    char temp[100] = "HHHH";
    //cout << "Please enter some info to add to the file" << endl;
    file.write((char*) &temp, 100);
    for(int i = 0; i < file.gcount(); i++){
        cout << temp[i];
    }
}

int main(int argc, char const *argv[])
{
    char buffer[BUF_SIZE];

    if (argc != 2){
        cout << "Program usage: prog_name file_name";
        return 1;
    }

    fstream file(argv[1], ios::in | ios::out | ios::binary | ios::app);
    if (!file){
        cout << "File can not open or doesn't exist";
        return 1;
    }

    //Try to read & write some info from/to file in particular position

    readFile(file, buffer);
    writeFile(file);

    file.close();

    return 0;
}

When I create a new ostream it works well, but I want to understand why fstream in/out mode works in my code only for reading.

cbuchart
  • 8,748
  • 6
  • 43
  • 72
Anton
  • 113
  • 1
  • 10

1 Answers1

3

I see several problems:

  1. The reason behind the writing problem is probably because you reach the end of the file (is the file smaller than BUF_SIZE bytes?). This sets the EOF bit, which makes any write operations to fail. You have to clear that bit before (use the std::fstream::clear method):

    void readFile(fstream &file, char buffer[BUF_SIZE])
    {
        int position;
        cout << "Please enter a position to read from the file some info" << endl;
        cin >> position;
        file.seekg(position, ios::beg);
        file.read(buffer, BUF_SIZE);
    
        for(int i = 0; i < file.gcount(); i++){
            cout << buffer[i];
        }
    
        file.clear(); // clears EOF
    }
    
  2. The line file.write((char*) &temp, 100); is wrong since you are actually passing a point to the temp variable, which is also a pointer, but it is camouflaged by the cast. These ones are OK: file.write(temp, 100); or file.write(&temp[0], 100);

  3. When printing the written characters, you are using std::fstream::gcount, which literally means get count (amount of characters read in the last get operation). You are writing (put) not reading (get). Indeed, you are actually indicating how many bytes you are willing to write, so use it:

    file.write(temp, 100);
    for(int i = 0; i < 100; i++){
        cout << temp[i];
    }
    
  4. Finally, you are always writing 100 characters, probably including some garbage from the buffer. As I see that you want to let the user choose what to write (the commented line), you can instead:

    const size_t size = strlen(temp);
    file.write(temp, size);
    for(size_t i = 0; i < size; i++){
        cout << temp[i];
    }
    

In addition, some suggestions:

  1. Use a std::string to read the user input, in this way you avoid a possible buffer overflow (if the user enters more than 100 characters).

    cin.ignore(numeric_limits<streamsize>::max(), '\n'); // read the link bel
    string temp;
    getline(cin, temp); // need #include <string>
    file.write(temp.c_str(), temp.size());
    

    You will probably want to read this answer to learn more about the first line (basically it avoids the getline to be skipped after using cin >> position).

  2. Avoid the for loop to print the user input. For both the buffer and the std::string options you can just cout << temp << endl;.

cbuchart
  • 8,748
  • 6
  • 43
  • 72
  • Thank you for the detailed response, cause Im in process of studying. 1. Can you clarify what operation set EOF bit? istream.read? I thought that there are two independent pointers - one for read and another for write operations (as seekp and seekg) is it not right think? – Anton Jun 02 '17 at 16:58
  • Yes, `stream::read`; although pointers are independent the state of the file is shared and checked by both `read` and `write`. – cbuchart Jun 02 '17 at 17:09
  • 2. Ok! Some programmer dude told me about this in comments too 3. Ok! 4. All of other recomendation is about using string type, and I find it more useful here too, THNX Best regards Anton – Anton Jun 02 '17 at 17:11
  • **"the state of the file is shared and checked by both read and write"**this is the real answer for my question – Anton Jun 02 '17 at 17:24
  • NOTE!: 1. 'file.write(temp, 100);' does not work in gcc, use only 'file.write(&temp[0], 100);' variant 2. 'const size_t size = strlen(temp);' does not work too, use 'const size_t STR_SIZE = temp.size();' [link](https://stackoverflow.com/questions/22313263/error-cannot-convert-const-string-to-const-char-for-argument-1-to-size-t-strle) – Anton Jun 05 '17 at 05:51
  • @Anton, correct, I checked it only for VS, and in the case of the second variant, of course, if you switched to `std::string` obviously the `strlen` won't work anymore (actually, the second example already uses it, but wasn't commented: `file.write(temp.c_str(), temp.size());`). Thanks for pointing these out! – cbuchart Jun 05 '17 at 06:44