[GitHub] [mynewt-core] kasjer commented on a change in pull request #2300: TCP/IPv4 adaptor for CoAP

[GitBox <gi...@apache.org>]

kasjer commented on a change in pull request #2300:
URL: https://github.com/apache/mynewt-core/pull/2300#discussion_r432281075



##########
File path: net/oic/include/oic/port/mynewt/stream.h
##########
@@ -0,0 +1,95 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/**
+ * stream.h: Utility code for adaptors that implement the CoAP-over-TCP
+ * protocol (Bluetooth, TCP/IP, etc.).
+ */
+
+#ifndef H_MYNEWT_TCP_
+#define H_MYNEWT_TCP_
+
+#include "os/mynewt.h"
+#include "oic/oc_ri.h"
+#include "oic/oc_ri_const.h"
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/**
+ * Indicates whether a transport-specific endpoint matches the provided
+ * description.
+ */
+typedef bool oc_stream_ep_match(const void *ep, void *ep_desc);

Review comment:
       Any reason why only ep is const?

##########
File path: net/oic/src/port/mynewt/tcp4_adaptor.c
##########
@@ -0,0 +1,425 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include <assert.h>
+#include <string.h>
+#include <stdint.h>
+
+#include "os/mynewt.h"
+
+uint8_t oc_tcp4_transport_id = -1;

Review comment:
       Slightly unorthodox way to initialize unsigned value.
   Other place in this commit use UINT8_MAX

##########
File path: net/oic/src/port/mynewt/stream.c
##########
@@ -0,0 +1,112 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/**
+ * stream.c: Utility code for adaptors that implement the CoAP-over-TCP
+ * protocol (Bluetooth, TCP/IP, etc.).
+ */
+
+#include "os/mynewt.h"
+#include "oic/port/mynewt/stream.h"
+#include "oic/messaging/coap/coap.h"
+
+int
+oc_stream_reass(struct oc_stream_reassembler *r, struct os_mbuf *om1,
+                void *ep_desc, struct os_mbuf **out_pkt)
+{
+    struct os_mbuf_pkthdr *pkt1;
+    struct os_mbuf_pkthdr *pkt2;
+    struct os_mbuf *om2;
+    uint8_t hdr[sizeof (struct coap_tcp_hdr32)];

Review comment:
       This is the only place where space is after sizeof

##########
File path: net/oic/src/port/mynewt/tcp4_adaptor.c
##########
@@ -0,0 +1,425 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include <assert.h>
+#include <string.h>
+#include <stdint.h>
+
+#include "os/mynewt.h"
+
+uint8_t oc_tcp4_transport_id = -1;
+
+#if MYNEWT_VAL(OC_TRANSPORT_TCP4)
+
+#include <log/log.h>
+#include <mn_socket/mn_socket.h>
+#include <stats/stats.h>
+
+#include "oic/oc_log.h"
+#include "oic/port/oc_connectivity.h"
+#include "oic/port/mynewt/adaptor.h"
+#include "oic/port/mynewt/transport.h"
+#include "oic/port/mynewt/ip.h"
+#include "oic/port/mynewt/stream.h"
+#include "oic/port/mynewt/tcp4.h"
+
+#ifdef OC_SECURITY
+#error This implementation does not yet support security
+#endif
+
+static void oc_send_buffer_tcp4(struct os_mbuf *m);
+static uint8_t oc_ep_tcp4_size(const struct oc_endpoint *oe);
+static char *oc_log_ep_tcp4(char *ptr, int maxlen, const struct oc_endpoint *);
+static int oc_connectivity_init_tcp4(void);
+void oc_connectivity_shutdown_tcp4(void);
+static void oc_event_tcp4(struct os_event *ev);
+static void oc_tcp4_readable(void *cb_arg, int err);
+static void oc_tcp4_writable(void *cb_arg, int err);
+static struct oc_tcp4_conn *oc_tcp4_remove_conn(struct mn_socket *sock);
+static bool oc_tcp4_ep_match(const void *ep, void *arg);
+static void oc_tcp4_ep_fill(void *ep, void *arg);
+
+static const struct oc_transport oc_tcp4_transport = {
+    .ot_flags = OC_TRANSPORT_USE_TCP,
+    .ot_ep_size = oc_ep_tcp4_size,
+    .ot_tx_ucast = oc_send_buffer_tcp4,
+    .ot_tx_mcast = NULL,
+    .ot_get_trans_security = NULL,
+    .ot_ep_str = oc_log_ep_tcp4,
+    .ot_init = oc_connectivity_init_tcp4,
+    .ot_shutdown = oc_connectivity_shutdown_tcp4
+};
+
+static struct os_event oc_tcp4_read_event = {
+    .ev_cb = oc_event_tcp4,
+};
+
+#define COAP_PORT_UNSECURED (5683)
+
+STATS_SECT_START(oc_tcp4_stats)
+    STATS_SECT_ENTRY(iframe)
+    STATS_SECT_ENTRY(ibytes)
+    STATS_SECT_ENTRY(ierr)
+    STATS_SECT_ENTRY(oucast)
+    STATS_SECT_ENTRY(obytes)
+    STATS_SECT_ENTRY(oerr)
+STATS_SECT_END
+STATS_SECT_DECL(oc_tcp4_stats) oc_tcp4_stats;
+STATS_NAME_START(oc_tcp4_stats)
+    STATS_NAME(oc_tcp4_stats, iframe)
+    STATS_NAME(oc_tcp4_stats, ibytes)
+    STATS_NAME(oc_tcp4_stats, ierr)
+    STATS_NAME(oc_tcp4_stats, oucast)
+    STATS_NAME(oc_tcp4_stats, obytes)
+    STATS_NAME(oc_tcp4_stats, oerr)
+STATS_NAME_END(oc_tcp4_stats)
+
+/**
+ * Describes a tcp endpoint.  EP descriptors get passed to callbacks that need
+ * to operate on a specific managed connection.
+ */
+struct oc_tcp4_ep_desc {
+    struct mn_socket *sock;
+    oc_ipv4_addr_t addr;
+    uint16_t port;
+};
+
+static struct oc_stream_reassembler oc_tcp4_r = {
+    .pkt_q = STAILQ_HEAD_INITIALIZER(oc_tcp4_r.pkt_q),
+    .ep_match = oc_tcp4_ep_match,
+    .ep_fill = oc_tcp4_ep_fill,
+    .endpoint_size = sizeof(struct oc_endpoint_tcp),
+};
+
+struct oc_tcp4_conn {
+    struct mn_socket *sock;
+    oc_tcp4_err_fn *err_cb;
+    void *err_arg;
+    SLIST_ENTRY(oc_tcp4_conn) next;
+};
+
+struct os_mempool oc_tcp4_conn_pool;
+static os_membuf_t oc_tcp4_conn_buf[
+    OS_MEMPOOL_SIZE(MYNEWT_VAL(OC_TCP4_MAX_CONNS),
+                    sizeof (struct oc_tcp4_conn))

Review comment:
       Second place with space after sizeof few lines above there was no space




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org