How to get rid of -Wpointer-arith

Question

This code:

void Pack::packUInteger(void **buffer, unsigned int payload){
    memcpy(*buffer, &payload, sizeof(unsigned int));
    *buffer += sizeof(unsigned int);    
}

yields this warning, that I would like to get rid off without telling the compiler to ignore it:

src/messaging/Pack.cpp: In static member function ‘static void Pack::packUInteger(void**, unsigned int)’:
src/messaging/Pack.cpp:33:10: warning: pointer of type ‘void *’ used in arithmetic [-Wpointer-arith]
  *buffer += sizeof(unsigned int);
  ~~~~~~~~^~~~~~~~~~

I know it should there needs to be a de-referencing and casting, but I can't figure out exactly how to do it correctly.

Thank you internet! :)

eerorika · Accepted Answer · 2019-03-21T11:34:40.597

2

It's impossible to verify that this is correct unless you show what the pointer is pointing at.

But given that you attempt to increment the pointer by sizeof(unsigned int), it would make sense if *buffer points to an element of an array of unsigned int, and you attempt to increment the pointer to the next sibling.

A correct way to do that is:

auto ptr = static_cast<unsigned*>(*buffer);
*buffer = ptr + 1;

On the other hand, if if points to raw storage such as std::byte, a correct way is:

auto ptr = static_cast<std::byte*>(*buffer);
*buffer = ptr + sizeof payload;

Instead of using void**, I recommend following instead:

template <class T>
std::byte* pack(std::byte* buffer, T payload) {
    static_assert(std::is_trivially_copyable_v<T>);
    std::memcpy(buffer, std::addressof(payload), sizeof payload);
    return buffer + sizeof payload;
}

edited Mar 21 '19 at 11:34

answered Mar 20 '19 at 14:33

eerorika

232,697
12
197
326

I should have mentioned I don't have access to std::, as the code is supposed to run in Intel SGX enclaves :) Other than that, your answer was really helpful! Thank you! – larslars Mar 21 '19 at 12:39
@larslars None of `std::` stuff are absolutely necessary here. Use `unsigned char` instead of `std::byte`. Include `string.h` for `::memcpy`. You can implement a `nonstd::addressof` easily yourself. `std::is_trivially_copyable` is probably non-trivial to implement, but the assertion can be removed and the requirement documented separately. – eerorika Mar 21 '19 at 12:49
Yes thanks it does work without it. We have our own memcpy and I'm considering an addressof implementation. I will be adding sanity checks (memory bounds, range checking etc) as we go, but I will have to think more about is_trivially_copyable :) – larslars Mar 21 '19 at 12:56

score 1 · Answer 2 · answered Mar 20 '19 at 14:32

Well you're incrementing a void pointer.
What is the size of void??

You should cast the pointer to the correct type before incrementing it.
sizeof gives the size in bytes so the correct type should be uint8_t or unsigned char.

*buffer = (uint8_t*)(*buffer) + sizeof(unsigned int);

How to get rid of -Wpointer-arith

2 Answers2