Find a number in a binary file, rotate and overwrite

Question

I'm trying to find an integer (6664) in a binary file (file.bin) and I must shift this integer to the right once and write this result to the same position the integer was found. However, I am not able to overwrite the original value with the shifted value. I developed the code below:

#include <cstdio>
#include <cstring>
int main(){

    
    //opening file
    FILE *pfile
    pfile = fopen("file.bin","r+b");

    //ffinding file size
    fseek(pfile, 0L, SEEK_END);
    long size= ftell(pfile);
    rewind(pfile);
    //reading data
    int vec[size];
    fread(vec, sizeof(int), size, pfile);
    
    for(int i = 0; i<size; i++){
         if(vec[i]==6664){
             int aux = vec[i]>>1;
             fseek(pfile, i, SEEK_SET);
             fwrite(&aux, sizeof(int), 1, pfile);
            
     }
        }
        
    
    
    return 0;
}

I generated the file with the following command:

echo "0000000: 6408 0623 77ef bfbd efbf bdef bfbd 2779 d..#w.........'y
0000010: efbf bdef 081a 0000 0000 efbf bdef bfbd ................
0000020: 4577 efbf bdef bfbd efbf bd00 Ew.........." | xxd -r > file.bin

Any thoughts?

Answer 1

IMHO, you should read in chunks. There is no guarantee that your platform or executable has enough memory to read in the file (files can be huge).

static const size_t QUANTITY_INTEGERS = 1024u * 1024u;
uint16_t buffer[QUANTITY_INTEGERS] = {0};
std::ifstream number_file("file.bin", ios::binary);
while (number_file.read((char *) &buffer[0], QUANTITY_INTEGERS * sizeof(uint16_t)))
{
    unsigned int numbers_read = number_file.gcount();
    uint16_t * p_number = std::find(&buffer[0], &buffer[numbers_read], 0x6664);
    if (p_number != &buffer[numbers_read])
    {
         std::cout << "0x6664 found.\n";
         break;
    }
}

You can use whatever buffer capacity you want.

Answer 2

Aside from the complete lack of error handling, you are not adequately differentiating between bytes and integers.

Your size variable is expressed in bytes, but you are using it as a count when dealing with your vec array. As such, you are over-allocating your array, reading too many ints from the file into the array, and iterating through too many elements of the array.

Also, when writing back to the file, you are treating the loop counter i as a byte offset, which it is not, thus you are writing to the wrong offset in the file.

Try something more like this instead:

#include <cstdio>
#include <cstdint>
#include <vector>

typedef int32_t myint_t; // or int16_t, if needed...

int main()
{
    //opening file
    FILE *pfile = fopen("file.bin", "r+b");
    if (!pFile) return -1

    //finding file size
    //TODO: use f/stat() or std::filesystem::file_size() instead
    if (fseek(pfile, 0L, SEEK_END) < 0) {
        fclose(pFile);
        return -1;
    }
    long size = ftell(pfile);
    if (size < 0) {
        fclose(pFile);
        return -1;
    }
    rewind(pfile);

    //reading data
    std::vector<myint_t> vec(size / sizeof(myint_t));
    size_t numItems = fread(vec.data(), sizeof(myint_t), vec.size(), pfile);
    
    for(size_t i = 0; i < numItems; ++i) {
        if (vec[i] == 6664) {
            vec[i] >>= 1;
            // TODO: it is more efficient for the filesystem to make
            // smaller relative seeks using SEEK_CUR than to make
            // larger seeks from the beginning using SEEK_SET...
            if (fseek(pfile, i * sizeof(myint_t), SEEK_SET) < 0) {
                fclose(pFile);
                return -1;
            }
            if (fwrite(&vec[i], sizeof(myint_t), 1, pfile) != 1) {
                fclose(pFile);
                return -1;
            }
        }
    }
        
    fclose(pfile);

    return 0;
}

That being said, consider using C++-style file I/O instead of C-style file I/O, eg:

#include <iostream>
#include <vector>
#include <cstdint>

using myint_t = int32_t; // or int16_t, if needed...

int main()
{
    std::fstream fs;
    fs.exceptions(std::fstream::failbit | std::fstream::badbit);

    try {
        //opening file
        fs.open("file.bin", std::ios::in | std::ios::out | std::ios::binary | std::ios::ate);

        //finding file size
        //TODO: use f/stat() or std::filesystem::file_size() instead
        long size = fs.tellg();
        fs.seekg(0);

        //reading data
        std::vector<myint_t> vec(size / sizeof(myint_t));
        fs.read(reinterpret_cast<char*>(vec.data()), sizeof(myint_t) * vec.size());
        size_t numItems = fs.gcount() / sizeof(myint_t);

        for(size_t i = 0; i < numItems; ++i) {
            if (vec[i] == 6664) {
                vec[i] >>= 1;
                // TODO: it is more efficient for the filesystem to make
                // smaller relative seeks using std::ios::cur than to make
                // larger seeks from the beginning...
                fs.seekp(i * sizeof(myint_t));
                fs.write(reinterpret_cast<char*>(&vec[i]), sizeof(myint_t));
            }
        }
    }
    catch(...) {
        return -1;
    }

    return 0;
}

Though, the following would be a little bit simpler and safer:

#include <iostream>
#include <vector>
#include <filesystem>
#include <cstdint>

using myint_t = int32_t; // or int16_t, if needed...

int main()
{
    std::ifstream ifs;
    std::ofstream ofs;

    ifs.exceptions(std::ifstream::failbit | std::ifstream::badbit);
    ofs.exceptions(std::ofstream::failbit | std::ofstream::badbit);

    try {
        //opening files
        ifs.open("file.bin", std::ios::binary);
        ofs.open("file.new", std::ios::binary);

        //reading data
        myint_t value;
        bool found = false;
        while (ifs.read(reinterpret_cast<char*>(&value), sizeof(value))) {
            if (value == 6664) {
                value >>= 1;
                found = true;
            }
            ofs.write(reinterpret_cast<char*>(&value), sizeof(value));
        }

        ifs.close();
        ofs.close();

        if (found) {
            std::filesystem::rename("file.bin", "file.bak");
            std::filesystem::rename("file.new", "file.bin");
            std::filesystem::remove("file.bak");
        }
        else {
            std::filesystem::remove("file.new");
        }
    }
    catch(...) {
        return -1;
    }

    return 0;
}