Vincent Kenbeek
Vincent Kenbeek

Reputation: 617

Pointer changes on last iteration of foor loop

I'm trying to implement a streaming ACN receiver in C++. For this I have created a socket which receives data from a test source I got from the internet but runs locally. I receive the data just fine. However when I loop through the individual bytes of the vector in the root layer, the pointer I use suddenly changes on the last iteration of the for loop.

The code I use for checking the fields in the root layer looks as follows.

// Locations for each field of the root layer in the packet in the raw data
char *preamble_size_p    = &this->raw[0];
char *postamble_size_p   = &this->raw[2];
char *acn_pid_p          = &this->raw[4];
char *flags_and_length_p = &this->raw[16];
char * const vector_p    = &this->raw[18];

int i;
uint8_t acn_pid_byte;

// get preamble size and check it
memcpy(&this->packet.root_layer.preamble_size, preamble_size_p, 2);
this->packet.root_layer.preamble_size = htons(this->packet.root_layer.preamble_size);
if (this->packet.root_layer.preamble_size != PREAMBLE_SIZE)
    return PACKET_ERROR_INVALID_PREAMBLE_SIZE;

// get postamble size and check it
memcpy(&this->packet.root_layer.postamble_size, postamble_size_p, 2);
this->packet.root_layer.postamble_size = htons(this->packet.root_layer.postamble_size);
if (this->packet.root_layer.postamble_size != POSTAMBLE_SIZE)
    return PACKET_ERROR_INVALID_POSTAMBLE_SIZE;

// get ACN pid and check if valid
for (i = 0; i < ACN_PID_SIZE; i++) {
    memcpy(&acn_pid_byte, acn_pid_p + i, 1);
    if (acn_pid_byte != ACN_PID[i]) {
        return PACKET_ERROR_INVALID_ACN_PID;
    }
    this->packet.root_layer.acn_pid[i] = acn_pid_byte;
}

// get flags and length
memcpy(&this->packet.root_layer.flength, flags_and_length_p, 2);
this->packet.root_layer.flength = htons(this->packet.root_layer.flength);

/* ERROR HAPPENS IN LOOP BELOW */

// get vector and check if valid
uint32_t vector_bytes;
for (int k = 0; k < 4; k++) {
    memcpy(&vector_bytes + k, vector_p + k, 1);
}

As you can see I made the vector pointer a constant. It used to be a regular pointer like the others. Even though it is a constant it still changes value.

raw is just char *raw which is assigned in the constructor of the class and contains the bytes my program receives from the source.

char *raw;  
int size;

Packet(char *raw, int size) {
    this->raw = raw;
    this->size = size;
}

The code for receiving the data.

char buffer[MAXLINE] = {0}; // MAXLINE is 1024
n = recvfrom(sock_fd, (char *)buffer, MAXLINE, MSG_WAITALL, ( struct sockaddr *) &cli_addr, &len);
Packet packet = Packet(buffer, n);

Immediately after this packet.process() is called which starts running the code in the top code block.

I have tried setting a watchpoint in gdb and this is the result.

Breakpoint 1, Packet::processRootLayer (this=0x7fffffffd5e0) at server.cpp:45
45          memcpy(&this->packet.root_layer.preamble_size, preamble_size_p, 2);
(gdb) watch vector_p
Hardware watchpoint 2: vector_p
(gdb) c
Continuing.

Hardware watchpoint 2: vector_p

Old value = 0x7fffffffdcc2 ""
New value = 0x7fffffffdc00 ""
Packet::processRootLayer (this=0x7fffffffd5e0) at server.cpp:71
71          for (int k = 0; k < 4; k++) {
(gdb) 

Any help would be appreciated and if you need more information please let me know.

Upvotes: 0

Views: 93

Answers (1)

john
john

Reputation: 88027

Your vector_bytes variable is typed as uint32_t so &vector_bytes + k does not add k bytes to &vector_bytes, it adds k*sizeof(uint32_t) bytes. As a result your memcpy is overwriting other variables in your program.

You could add a cast, like this

uint32_t vector_bytes;
for (int k = 0; k < 4; k++) {
    memcpy((char*)&vector_bytes + k, vector_p + k, 1);
}

or you could do it without a loop.

uint32_t vector_bytes;
memcpy(&vector_bytes, vector_p, 4);

Much simpler.

Upvotes: 3

Related Questions