0

I am using a lib which can load BMP images from memory.

I have a class which represents a BMP.

To load from memory I have to supply a pointer to some BMP formatted data in memory and a variable for the size of that data. (void* data, size_t length)

I want to store my data in a std::vector. (Avoids manual memory management)

I've attempted to write a function to return a std::vector<unsigned char>, but I don't think what I've got is very good.

std::vector<unsigned char> BMP::BITMAP::SaveMem() const
{

    // memory storage
    std::vector<unsigned char> memory;


    BITMAPFILEHEADER f_head;
    f_head.bfType = ushort_rev(((WORD)'B' << 0x08) | ((WORD)'M' << 0x00));
    f_head.bfSize = sizeof(BITMAPFILEHEADER) + sizeof(BITMAPINFOHEADER) + m_width_memory * m_height;
    f_head.bfReserved1 = 0;
    f_head.bfReserved2 = 0;
    f_head.bfOffBits = sizeof(BITMAPFILEHEADER) + sizeof(BITMAPINFOHEADER);


    // build standard bitmap file header
    BITMAPINFOHEADER i_head;
    i_head.biSize = sizeof(BITMAPINFOHEADER);
    i_head.biWidth = m_width;
    i_head.biHeight = m_height;
    i_head.biPlanes = 1;
    i_head.biBitCount = m_bit_count;
    i_head.biCompression = 0;
    i_head.biSizeImage = m_width_memory * m_height;
    i_head.biXPelsPerMeter = 0;
    i_head.biYPelsPerMeter = 0;
    i_head.biClrUsed = 0;
    i_head.biClrImportant = 0;


    // alloc
    memory.resize(f_head.bfSize);

    std::copy(&f_head, &f_head + sizeof(f_head), memory.at(0));
    std::copy(&i_head, &i_head + sizeof(i_head), memory.at(0) + sizeof(f_head));


    // write data
    for(unsigned int y = 0; y < m_height; ++ y)
    {
        std::copy(&m_data[y * m_width_memory], m_data[y * m_width_memory + 3 * m_size_x], memory.at(0) + sizeof(f_head) + sizeof(i_head));
    }

}

Clearly this doesn't compile. I can't think of any alternative to std::copy. Is this really the right tool for the job?

To make it compile I think I should change memory.at(x) to memory.data() + x... By doing this I would be using raw pointers - which is why I don't think std::copy is any better than memcpy.

Could I have some advice on this? It's somewhat an illogical task and had I known about this requirement earlier I would have stored my pixel data in an unsigned char with the bitmap file headers preceeding the data. Unfortunatly it will be a lot of work to change the design now, so I'd rather not touch it.

Ulrich Eckhardt
  • 16,572
  • 3
  • 28
  • 55
FreelanceConsultant
  • 13,167
  • 27
  • 115
  • 225
  • Is changing the design that terrible? How much time would it take to fix a bug because of this faulty design? – JVApen Dec 25 '18 at 21:50
  • @JVApen It's not ideal and will probably introduce a lot more bugs, since there's loads of code which depends on manipulating the BMP data at binary level – FreelanceConsultant Dec 25 '18 at 21:51

1 Answers1

1

Three problems:

  1. You want to copy bytes, but the std::copy function is provided a pointer to a BITMAPFILEHEADER (or BITMAPINFOHEADER) structure. You need to convert the pointers to bytes, like reinterpret_cast<uint8_t*>(&f_head).

  2. The previous leads to other problems with the end of the data, the expression &f_head + sizeof(f_head) which really is equal to (&f_head)[sizeof(f_head)], and is way beyond the end of the structure. You need to use bytes here as well, as in reinterpret_cast<uint8_t*>(&f_head) + sizeof f_head.

  3. The last problem is the destination for the std::copy call, as it needs to be a similar type as the source, i.e. a pointer to uint8_t (in the case of my casts). You can easily get that by doing e.g. &memory[0]. And for the second call &memory[sizeof f_head].

Some programmer dude
  • 400,186
  • 35
  • 402
  • 621
  • He's using `unsigned char` not `uint8_t`. – Galik Dec 25 '18 at 22:01
  • Also, I am sure a lot of code probably assumes this but can it be absolutely relied upon that each field of the struct will follow directly from the previous field with no padding? And can we be certain that `sizeof(i_head)` will not contain padding? – Galik Dec 25 '18 at 22:05
  • @Galik IIRC those structures are are designed and laid out to avoid padding, since they are (and have always been) used to read and write raw binary from and to files. – Some programmer dude Dec 25 '18 at 22:19
  • @Galik Good question: the struct actually has: `__attribute__((packed))` – FreelanceConsultant Dec 25 '18 at 23:00
  • Source and destination of std::cooy need not be similar, they just both need to be iterators. – n. m. could be an AI Dec 26 '18 at 11:38