0

Hello I am trying to write 8 bits from std::vector to binary file and read them back . Writing works fine , have checked with binary editor and all values are correct , but once I try to read I got bad data . Data that i am writing :

11000111 //bits

Data that i got from reading:

11111111 //bits

Read function :

std::vector<bool> Read()
{
    std::vector<bool> map;
    std::ifstream fin("test.bin", std::ios::binary);
    int size = 8 / 8.0f;
    char * buffer = new char[size];
    fin.read(buffer, size);
    fin.close();
    for (int i = 0; i < size; i++)
    {
        for (int id = 0; id < 8; id++)
        {
            map.emplace_back(buffer[i] << id);
        }
    }
    delete[] buffer;
    return map;
}

Write function(just so you guys know more whats going on)

void Write(std::vector<bool>& map) 
{
    std::ofstream fout("test.bin", std::ios::binary);
    char byte = 0;
    int byte_index = 0;
    for (size_t i = 0; i < map.size(); i++)
    {
        if (map[i]) 
        {
            byte |= (1 << byte_index);
        }
        byte_index++;
        if (byte_index > 7)
        {
            byte_index = 0;
            fout.write(&byte, sizeof(byte));
        }
    }
    fout.close();
}
Egi Dijus
  • 25
  • 2
  • 9
  • 11
    `int size = 8 / 8.0f;` -- the mind boggles. – Kerrek SB Jan 25 '17 at 20:46
  • size is for future use when i will be writing more then 8 bits , so by / 8.0f i am converting them to bytes – Egi Dijus Jan 25 '17 at 20:48
  • 1
    Not sure why you're using a fixed-size character buffer, especially one that's...a single byte long? Maybe? Are you expecting the file to contain 7.3 bits in the future? – tadman Jan 25 '17 at 20:48
  • 2
    `vector ` is not exactly the greatest thing to use for any purpose –  Jan 25 '17 at 20:48
  • @tadman i just trying to get everything running before use on my project where is 560k bits – Egi Dijus Jan 25 '17 at 20:49
  • 6
    a "std::vector" with the variable name "map" is basically asking your coworkers to hate you – RyanP Jan 25 '17 at 20:49
  • Bits are either there or not there. Why are you explicitly declaring the divisor as a float? [Figure out how large the file is](http://stackoverflow.com/questions/5840148/how-can-i-get-a-files-size-in-c) and allocate your buffer accordingly. – tadman Jan 25 '17 at 20:50
  • @tadman 1) that would not solve problem 2) i dont see reason for doing this because of my future use , i dont need to check file size because all files will be fixed size 700*800 bits – Egi Dijus Jan 25 '17 at 20:52
  • 1
    There's literally no reason to divide by a float. Also assumptions like #2 are how you get cripplingly bad overflow bugs. You can design around those assumptions, but always handle exceptional cases. – tadman Jan 25 '17 at 20:53

1 Answers1

3

Your code spreads out one byte (the value of buffer[i], where i is always 0) over 8 bools. Since you only read one byte, which happens to be non-zero, you now end up with 8 trues (since any non-zero integer converts to true).

Instead of spreading one value out, you probably want to split one value into its constituent bits:

for (int id = 0; id < 8; id++)
{
    map.emplace_back((static_cast<unsigned char>(buffer[i]) & (1U << id)) >> id);
}
Kerrek SB
  • 464,522
  • 92
  • 875
  • 1,084
  • thats because i pack 8 bits into one byte when was writing data to file – Egi Dijus Jan 25 '17 at 20:53
  • @EgiDijus: So you also need to split your one byte back out into eight bits. I added some code. – Kerrek SB Jan 25 '17 at 20:55
  • thats what i am trying to do when reading into loops buffer[i] << id , bit shifting byte to get 8 bits – Egi Dijus Jan 25 '17 at 20:58
  • 2
    @EgiDijus: Shifting isn't enough. You also need to mask out the unwanted bits. – Kerrek SB Jan 25 '17 at 20:58
  • Kerrek is right. Aside from the unsigned issues (never shift signed values), if you take `3` and shift it left by `1` you get a true value, shift it by `2` you still get a true value, etc. even though only the lower two bits are set. – Cameron Jan 25 '17 at 21:01
  • Just one more question why u casting as unsigned char if map is std::vector ? – Egi Dijus Jan 25 '17 at 21:02
  • @EgiDijus: Because you don't want to perform bitwise operations on signed types, and for some reason you didn't make your buffer unsigned. – Kerrek SB Jan 25 '17 at 21:03
  • i did normal char because read() function is asking me char not unsigned char – Egi Dijus Jan 25 '17 at 21:05
  • @EgiDijus: Yes, you should probably cast that in the `read` call. – Kerrek SB Jan 25 '17 at 21:42