Question

Let's say I have a class that I don't own: DataBuffer. It provides various get member functions:

get(uint8_t *value);
get(uint16_t *value); 
...

When reading from a structure contained in this buffer, I know the order and size of fields, and I want to reduce the chance of future code changes causing an error:

struct Record 
{
    uint16_t Header;
    uint16_t Content;
}

void ReadIntoRecord(Record* r)
{
    DataBuffer buf( initialized from the network with bytes )
    buf.get(&r->Header); // Good!
    buf.get(&r->Content);
}

Then someone checks in a change to do something with the header before writing it:

    uint8_t customHeader;
    buf.get(&customHeader);  // Wrong, stopped reading after only 1 byte
    r->Header = customHeader + 1;
    buf.get(&r->Content);  // now we're reading from the wrong part of the buffer.

Is the following an acceptable way to harden the code against changes? Remember, I can't change the function names to getByte, getUShort, etc. I could inherit from DataBuffer, but that seems like overkill.

    buf.get(static_cast<uint16_t*>(&r->Header));  // compiler will catch incorrect variable type
    buf.get(static_cast<uint16_t*>(&r->Content))

Updated with not-eye-safe legacy code example:

       float dummy_float;
        uint32_t dummy32;
        uint16_t dummy16;
        uint8_t dummy8;

        uint16_t headTypeTemp;
        buf.get(static_cast<uint16_t*>(&headTypeTemp));
        m_headType = HeadType(headTypeTemp);
        buf.get(static_cast<uint8_t*>(&hid));
        buf.get(m_Name);
        buf.get(m_SerialNumber);


        float start;
        buf.get(static_cast<float*>(&start));
        float stop;
        buf.get(static_cast<float*>(&stop));


        buf.get(static_cast<float*>(&dummy_float));
        setStuffA(dummy_float);

        buf.get(static_cast<uint16_t*>(&dummy16));
        setStuffB(float(dummy16)/1000);

        buf.get(static_cast<uint8_t*>(&dummy8));    //reserved





        buf.get(static_cast<uint32_t*>(&dummy32));
        Entries().setStart( dummy32 );
        buf.get(static_cast<uint32_t*>(&dummy32));
        Entries().setStop( dummy32 );
        buf.get(static_cast<float*>(&dummy_float));
        Entries().setMoreStuff( dummy_float );

        uint32_t datalength;
        buf.get(static_cast<uint32_t*>(&datalength));

        Entries().data().setLength(datalength);

        RetVal ret = ReturnCode::SUCCESS;
        Entry* data_ptr = Entries().data().data();
        for (unsigned int i = 0; i < datalength && ret == ReturnCode::SUCCESS; i++)
        {
            ret = buf.get(static_cast<float*>(&dummy_float));
            data_ptr[i].FieldA = dummy_float;
        }

        for (unsigned int i = 0; i < datalength && ret == ReturnCode::SUCCESS; i++)
        {
            ret = buf.get(static_cast<float*>(&dummy_float));
            data_ptr[i].FieldB = dummy_float;
        }

        // Read in the normalization vector
        Util::SimpleVector<float> norm;
        buf.get(static_cast<uint32_t*>(&datalength));
        norm.setLength(datalength);
        for (unsigned int i=0; i<datalength; i++)
        {
            norm[i] = buf.getFloat();
        }

        setNormalization(norm);

        return ReturnCode::SUCCESS;
}
Was it helpful?

Solution

Don't use overloading. Why not have get_word and get_dword calls? The interface isn't going to be any uglier but at least the mistake is a lot harder to make.

OTHER TIPS

wouldn't it be better to read the whole struct from the network? Letting the user do all the socket operations seems like a bad idea to me (not encapsulated). Encapsulate the stuff you want to send on the network to operate on file descriptors instead of letting the user put raw buffer data to the file descriptors.

I can imagine something like

void readHeader(int filedes, struct Record * Header);

so you can do something like this

struct Record 
{
  uint16_t Header;
  uint16_t Content;
  uint16_t getHeader() const { return Header; }
  uint16_t getContent() const  { return Content; }  
};

/* socket stuff to get filedes */
struct Record x;
readHeader(fd, &x);
x.getContent();

You can't read from buffer with type safety unless the buffer contains information about the content. One simple method is to add length to each structure and check that at least the data being read is still the sane length. You could also use XML or ASN.1 or something similar where type information is provided. Of course I'm assuming that you also write to that buffer.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top