> On 1 Mar 2018, at 11:02, Lukáš Hrázký <lhrazky@xxxxxxxxxx> wrote:
>
> On Wed, 2018-02-28 at 16:43 +0100, Christophe de Dinechin wrote:
>> From: Christophe de Dinechin <dinechin@xxxxxxxxxx>
>>
>> Use C++ "placement new" to directly initialize a C++ object at a
>> specified memory location. This makes it possible to get rid of
>> type-unsafe memset, low-level unsafe pointer + sizeof displacement
>> computations, and type-unsafe reinterpret_cast<>.
>>
>> Signed-off-by: Christophe de Dinechin <dinechin@xxxxxxxxxx>
>> ---
>> src/spice-streaming-agent.cpp | 44 +++++++++++++++++++++++++------------------
>> 1 file changed, 26 insertions(+), 18 deletions(-)
>>
>> diff --git a/src/spice-streaming-agent.cpp b/src/spice-streaming-agent.cpp
>> index 5c36906..21f9c31 100644
>> --- a/src/spice-streaming-agent.cpp
>> +++ b/src/spice-streaming-agent.cpp
>> @@ -334,36 +334,44 @@ static void usage(const char *progname)
>> }
>>
>> static void
>> -send_cursor(int streamfd, unsigned width, unsigned height, int hotspot_x, int hotspot_y,
>> +send_cursor(int streamfd,
>> + uint16_t width, uint16_t height,
>> + uint16_t hotspot_x, uint16_t hotspot_y,
>> std::function<void(uint32_t *)> fill_cursor)
>> {
>> if (width >= STREAM_MSG_CURSOR_SET_MAX_WIDTH || height >= STREAM_MSG_CURSOR_SET_MAX_HEIGHT) {
>> return;
>> }
>>
>> - size_t cursor_size = sizeof(CursorMessage) + width * height * sizeof(uint32_t);
>> - std::unique_ptr<uint8_t[]> msg(new uint8_t[cursor_size]);
>> + const uint32_t msgsize = sizeof(CursorMessage) + width * height * sizeof(uint32_t);
>> + const uint32_t hdrsize = sizeof(StreamDevHeader);
>
> Double space. Why use uint32_t instead of size_t?
defined by the protocol.
>
> As I noted before, hdrsize is only used once, did you consider using
> sizeof(StreamDevHeader) in-place?
yes. the goal is to converge pieces of code that were doing the same thing differently, so I rewrote them to highlight similarities.
>
> Lukas
>
>> - StreamDevHeader &dev_hdr(*reinterpret_cast<StreamDevHeader*>(msg.get()));
>> - memset(&dev_hdr, 0, sizeof(dev_hdr));
>> - dev_hdr.protocol_version = STREAM_DEVICE_PROTOCOL;
>> - dev_hdr.type = STREAM_TYPE_CURSOR_SET;
>> - dev_hdr.size = cursor_size - sizeof(StreamDevHeader);
>> + std::unique_ptr<uint8_t[]> storage(new uint8_t[msgsize]);
>>
>> - StreamMsgCursorSet &cursor_msg(*reinterpret_cast<StreamMsgCursorSet *>(msg.get() + sizeof(StreamDevHeader)));
>> - memset(&cursor_msg, 0, sizeof(cursor_msg));
>> -
>> - cursor_msg.type = SPICE_CURSOR_TYPE_ALPHA;
>> - cursor_msg.width = width;
>> - cursor_msg.height = height;
>> - cursor_msg.hot_spot_x = hotspot_x;
>> - cursor_msg.hot_spot_y = hotspot_y;
>> + CursorMessage *cursor_msg =
>> + new(storage.get()) CursorMessage {
>> + .hdr = {
>> + .protocol_version = STREAM_DEVICE_PROTOCOL,
>> + .padding = 0, // Workaround GCC internal / not implemented compiler error
>> + .type = STREAM_TYPE_CURSOR_SET,
>> + .size = msgsize - hdrsize
>> + },
>> + .msg = {
>> + .width = width,
>> + .height = height,
>> + .hot_spot_x = hotspot_x,
>> + .hot_spot_y = hotspot_y,
>> + .type = SPICE_CURSOR_TYPE_ALPHA,
>> + .padding1 = { },
>> + .data = { }
>> + }
>> + };
>>
>> - uint32_t *pixels = reinterpret_cast<uint32_t *>(cursor_msg.data);
>> + uint32_t *pixels = reinterpret_cast<uint32_t *>(cursor_msg->msg.data);
>> fill_cursor(pixels);
>>
>> std::lock_guard<std::mutex> stream_guard(stream_mtx);
>> - write_all(streamfd, msg.get(), cursor_size);
>> + write_all(streamfd, storage.get(), msgsize);
>> }
>>
>> static void cursor_changes(int streamfd, Display *display, int event_base)
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel