search

mbroadst / qamqp

AMQP 0.9.1 implementation for Qt
Other
151 stars 127 forks source link

Use stack instead of broken usage of QByteBuffer #3

Closed AdamMajer closed 10 years ago

AdamMajer commented 10 years ago

Buffer should not be used blindly when reading Frame::HEADER_SIZE. It's error prone and adds nothing over reading to stack - and it's only 7 bytes. There are places in this class where buffer.clear() is called.

An issue that will need to be fixed is the breaking of strict-aliasing rules. That cast is clearly for unaligned int which will break on some platforms.

mbroadst commented 10 years ago

Agreed. The committed fix was a stopgap for the reported issue on Windows, however all of this code is the next step for cleanups in my optimization branches. In fact, in one of those branches I just grab all these values up front rather than passing them on to the various frame types (which each create their own QDataStream and read the values in there, in Frame::Base). It's still going to be a little bit before I get to those chances while I put together the benchmarking code itself, so if you have suggestions in the meantime they are more than welcome.

@MaplerStory can you verify this fixes your issues for the interim?

mbroadst commented 10 years ago

merged with 7ff0dfab1bcda8904a52c5682fe2ed4c52c5292e

MaplerStory commented 10 years ago

@mbroadst , yeah, i test the commit on windows. it works well