Fix review comments left from the initial commit

Added some minor improvements and fixes for review
comments from the initial commit.

Change-Id: Ic3b36118262660c110ccf7ef2209a25208703ab7
Reviewed-by: Alex Blasche <alexander.blasche@qt.io>
Reviewed-by: Karsten Heimrich <karsten.heimrich@qt.io>

7 files changed, 27 insertions, 18 deletions

diff --git a/src/coap/qcoapclient.cpp b/src/coap/qcoapclient.cpp
index fc5e66a..36b6888 100644
--- a/src/coap/qcoapclient.cpp
+++ b/src/coap/qcoapclient.cpp
@@ -32,6 +32,7 @@
 #include "qcoapreply.h"
 #include "qcoapdiscoveryreply.h"
 #include "qcoapnamespace.h"
+#include <QtCore/qiodevice.h>
 #include <QtCore/qurl.h>
 #include <QtNetwork/qudpsocket.h>
 
@@ -562,7 +563,7 @@ void QCoapClient::cancelObserve(QCoapReply *notifiedReply)
     Sends the CoAP \a request to its own URL and returns a new QCoapReply
     object.
 */
-QCoapReply *QCoapClientPrivate::sendRequest(QCoapRequest &request)
+QCoapReply *QCoapClientPrivate::sendRequest(const QCoapRequest &request)
 {
     Q_Q(QCoapClient);
 
@@ -583,7 +584,7 @@ QCoapReply *QCoapClientPrivate::sendRequest(QCoapRequest &request)
     Sends the CoAP \a request to its own URL and returns a
     new QCoapDiscoveryReply object.
 */
-QCoapDiscoveryReply *QCoapClientPrivate::sendDiscovery(QCoapRequest &request)
+QCoapDiscoveryReply *QCoapClientPrivate::sendDiscovery(const QCoapRequest &request)
 {
     Q_Q(QCoapClient);
 
diff --git a/src/coap/qcoapclient.h b/src/coap/qcoapclient.h
index 877e3e7..993399a 100644
--- a/src/coap/qcoapclient.h
+++ b/src/coap/qcoapclient.h
@@ -35,7 +35,6 @@
 #include <QtCoap/qcoapglobal.h>
 #include <QtCoap/qcoapnamespace.h>
 #include <QtCore/qobject.h>
-#include <QtCore/qiodevice.h>
 #include <QtNetwork/qabstractsocket.h>
 
 QT_BEGIN_NAMESPACE
@@ -45,6 +44,7 @@ class QCoapDiscoveryReply;
 class QCoapRequest;
 class QCoapProtocol;
 class QCoapConnection;
+class QIODevice;
 
 class QCoapClientPrivate;
 class Q_COAP_EXPORT QCoapClient : public QObject
diff --git a/src/coap/qcoapclient_p.h b/src/coap/qcoapclient_p.h
index c8c3d73..0ddf34a 100644
--- a/src/coap/qcoapclient_p.h
+++ b/src/coap/qcoapclient_p.h
@@ -61,8 +61,8 @@ public:
     QCoapConnection *connection = nullptr;
     QThread *workerThread = nullptr;
 
-    QCoapReply *sendRequest(QCoapRequest &request);
-    QCoapDiscoveryReply *sendDiscovery(QCoapRequest &request);
+    QCoapReply *sendRequest(const QCoapRequest &request);
+    QCoapDiscoveryReply *sendDiscovery(const QCoapRequest &request);
     bool send(QCoapReply *reply);
 
     Q_DECLARE_PUBLIC(QCoapClient)
diff --git a/src/coap/qcoapinternalrequest.cpp b/src/coap/qcoapinternalrequest.cpp
index b76f255..486e51a 100644
--- a/src/coap/qcoapinternalrequest.cpp
+++ b/src/coap/qcoapinternalrequest.cpp
@@ -132,7 +132,7 @@ void QCoapInternalRequest::initForReset(quint16 messageId)
     Explicitly casts \a value to a char and appends it to the \a buffer.
 */
 template<typename T>
-void appendByte(QByteArray *buffer, T value) {
+static void appendByte(QByteArray *buffer, T value) {
     buffer->append(static_cast<char>(value));
 }
 
@@ -173,13 +173,13 @@ QByteArray QCoapInternalRequest::toQByteArray() const
 
     // Insert Options
     if (!d->message.options().isEmpty()) {
-        // Sort options by ascending order
-        // TODO: sort at insertion time in QCoapMessage, and assert that options are sorted here
         QVector<QCoapOption> options = d->message.options();
-        std::sort(options.begin(), options.end(),
-            [](const QCoapOption &a, const QCoapOption &b) -> bool {
-                return a.name() < b.name();
-        });
+
+        // Options should be sorted in order of their option numbers
+        Q_ASSERT(std::is_sorted(d->message.options().cbegin(), d->message.options().cend(),
+                                [](const QCoapOption &a, const QCoapOption &b) -> bool {
+                                    return a.name() < b.name();
+                 }));
 
         quint8 lastOptionNumber = 0;
         for (const QCoapOption &option : qAsConst(options)) {
@@ -336,10 +336,10 @@ QCoapOption QCoapInternalRequest::blockOption(QCoapOption::OptionName name, uint
     QByteArray optionValue;
     Q_ASSERT(!(optionData >> 24));
     if (optionData > 0xFFFF)
-        optionValue.append(static_cast<char>(optionData >> 16));
+        appendByte(&optionValue, optionData >> 16);
     if (optionData > 0xFF)
-        optionValue.append(static_cast<char>((optionData >> 8) & 0xFF));
-    optionValue.append(static_cast<char>(optionData & 0xFF));
+        appendByte(&optionValue, (optionData >> 8) & 0xFF);
+    appendByte(&optionValue, optionData & 0xFF);
 
     return QCoapOption(name, optionValue);
 }
diff --git a/src/coap/qcoapmessage.cpp b/src/coap/qcoapmessage.cpp
index e7d87c7..0b7fd2a 100644
--- a/src/coap/qcoapmessage.cpp
+++ b/src/coap/qcoapmessage.cpp
@@ -132,7 +132,15 @@ void QCoapMessage::addOption(QCoapOption::OptionName name, const QByteArray &val
 void QCoapMessage::addOption(const QCoapOption &option)
 {
     Q_D(QCoapMessage);
-    d->options.push_back(option);
+
+    // Sort options by ascending order while inserting
+    d->options.insert(
+                std::upper_bound(d->options.begin(), d->options.end(), option,
+                                 [](const QCoapOption &a, const QCoapOption &b) -> bool {
+                                     return a.name() < b.name();
+                                 }),
+                option
+            );
 }
 
 /*!
diff --git a/src/coap/qcoapoption.cpp b/src/coap/qcoapoption.cpp
index 09c5dc8..b3058f5 100644
--- a/src/coap/qcoapoption.cpp
+++ b/src/coap/qcoapoption.cpp
@@ -48,7 +48,7 @@ QT_BEGIN_NAMESPACE
 
     Indicates the name of an option.
     The value of each ID is as specified by the CoAP standard, with the
-    exception of Invalid. You can refer to the
+    exception of Invalid. You can refer to
     \l{https://tools.ietf.org/html/rfc7252#section-5.10}{RFC 7252} and
     \l{https://tools.ietf.org/html/rfc7959#section-2.1}{RFC 7959} for more details.
 
diff --git a/src/coap/qcoapreply.h b/src/coap/qcoapreply.h
index 88b127c..bce03fe 100644
--- a/src/coap/qcoapreply.h
+++ b/src/coap/qcoapreply.h
@@ -47,7 +47,7 @@ class Q_COAP_EXPORT QCoapReply : public QIODevice
 public:
 
     explicit QCoapReply(const QCoapRequest &request, QObject *parent = nullptr);
-    ~QCoapReply();
+    ~QCoapReply() override;
 
     QtCoap::ResponseCode responseCode() const;
     QCoapMessage message() const;