Skip to content

Commit 49407e8

Browse files
hasumikinhasumikin
committed
Callers of {TCP|UDP}Client_send are responsible for freeing recv_data
1 parent 1500948 commit 49407e8

File tree

4 files changed

+65
-82
lines changed

4 files changed

+65
-82
lines changed

‎mrbgems/picoruby-net/include/net.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,8 @@ typedef struct {
3434
} net_response_t;
3535

3636
void DNS_resolve(const char *name, bool is_tcp, char *outbuf, size_t outlen);
37-
void TCPClient_send(mrb_state *mrb, const net_request_t *req, net_response_t *res);
38-
void UDPClient_send(mrb_state *mrb, const net_request_t *req, net_response_t *res);
37+
bool TCPClient_send(mrb_state *mrb, const net_request_t *req, net_response_t *res);
38+
bool UDPClient_send(mrb_state *mrb, const net_request_t *req, net_response_t *res);
3939

4040
void lwip_begin(void);
4141
void lwip_end(void);

‎mrbgems/picoruby-net/src/mruby/net.c‎

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -37,16 +37,15 @@ mrb_net_tcpclient_s__request_impl(mrb_state *mrb, mrb_value self)
3737
.is_tls = use_dtls
3838
};
3939
net_response_t res = {
40-
.recv_data = NULL,
40+
.recv_data = mrb_calloc(mrb, 1, 1),
4141
.recv_data_len = 0
4242
};
43-
TCPClient_send(mrb, &req, &res);
44-
if (res.recv_data == NULL) {
45-
ret = mrb_nil_value();
46-
} else {
43+
if (TCPClient_send(mrb, &req, &res)) {
4744
ret = mrb_str_new(mrb, (const char*)res.recv_data, res.recv_data_len);
48-
picorb_free(mrb, res.recv_data);
45+
} else {
46+
ret = mrb_nil_value();
4947
}
48+
mrb_free(mrb, res.recv_data);
5049
return ret;
5150
}
5251

@@ -67,16 +66,15 @@ mrb_net_udpclient_s__send_impl(mrb_state *mrb, mrb_value self)
6766
.is_tls = use_dtls
6867
};
6968
net_response_t res = {
70-
.recv_data = NULL,
69+
.recv_data = mrb_calloc(mrb, 1, 1),
7170
.recv_data_len = 0
7271
};
73-
UDPClient_send(mrb, &req, &res);
74-
if (res.recv_data == NULL) {
75-
ret = mrb_nil_value();
76-
} else {
72+
if (UDPClient_send(mrb, &req, &res) && res.recv_data) {
7773
ret = mrb_str_new(mrb, (const char*)res.recv_data, res.recv_data_len);
78-
picorb_free(mrb, res.recv_data);
74+
} else {
75+
ret = mrb_nil_value();
7976
}
77+
mrb_free(mrb, res.recv_data);
8078
return ret;
8179
}
8280

‎mrbgems/picoruby-net/src/tcp.c‎

Lines changed: 18 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -65,9 +65,6 @@ static err_t
6565
TCPClient_recv_cb(void *arg, struct altcp_pcb *pcb, struct pbuf *pbuf, err_t err)
6666
{
6767
tcp_connection_state *cs = (tcp_connection_state *)arg;
68-
printf("cs->state = %d\n", cs->state);
69-
printf("cs->pcb = %p\n", cs->pcb);
70-
printf("cs->send_data = %p, len = %zu\n", cs->send_data, cs->send_data_len);
7168
mrb_state *mrb = cs->mrb;
7269
if (err != ERR_OK) {
7370
picorb_warn("TCPClient_recv_cb: err=%d\n", err);
@@ -85,21 +82,15 @@ printf("cs->send_data = %p, len = %zu\n", cs->send_data, cs->send_data_len);
8582
current_pbuf = current_pbuf->next;
8683
}
8784
tmpbuf[pbuf->tot_len] = '\0';
88-
if (cs->recv_data == NULL) {
89-
cs->recv_data = picorb_alloc(mrb, pbuf->tot_len + 1);
90-
cs->recv_data_len = pbuf->tot_len;
91-
memcpy(cs->recv_data, tmpbuf, pbuf->tot_len);
92-
} else {
93-
char *new_recv_data = (char *)picorb_realloc(mrb, cs->recv_data, cs->recv_data_len + pbuf->tot_len + 1);
94-
if (new_recv_data == NULL) {
95-
picorb_free(mrb, tmpbuf);
96-
picorb_free(mrb, cs->recv_data);
97-
return ERR_MEM;
98-
}
99-
cs->recv_data = new_recv_data;
100-
memcpy(cs->recv_data + cs->recv_data_len, tmpbuf, pbuf->tot_len);
101-
cs->recv_data_len += pbuf->tot_len;
85+
assert(cs->recv_data);
86+
char *new_recv_data = (char *)picorb_realloc(mrb, cs->recv_data, cs->recv_data_len + pbuf->tot_len + 1);
87+
if (new_recv_data == NULL) {
88+
picorb_free(mrb, tmpbuf);
89+
return ERR_MEM;
10290
}
91+
cs->recv_data = new_recv_data;
92+
memcpy(cs->recv_data + cs->recv_data_len, tmpbuf, pbuf->tot_len);
93+
cs->recv_data_len += pbuf->tot_len;
10394
picorb_free(mrb, tmpbuf);
10495
altcp_recved(pcb, pbuf->tot_len);
10596
cs->state = NET_TCP_STATE_PACKET_RECVED;
@@ -121,9 +112,6 @@ static err_t
121112
TCPClient_connected_cb(void *arg, struct altcp_pcb *pcb, err_t err)
122113
{
123114
tcp_connection_state *cs = (tcp_connection_state *)arg;
124-
printf("cs->state = %d\n", cs->state);
125-
printf("cs->pcb = %p\n", cs->pcb);
126-
printf("cs->send_data = %p, len = %zu\n", cs->send_data, cs->send_data_len);
127115
mrb_state *mrb = cs->mrb;
128116
if (err != ERR_OK) {
129117
picorb_warn("TCPClient_connected_cb: err=%d\n", err);
@@ -137,9 +125,6 @@ static err_t
137125
TCPClient_poll_cb(void *arg, struct altcp_pcb *pcb)
138126
{
139127
tcp_connection_state *cs = (tcp_connection_state *)arg;
140-
printf("cs->state = %d\n", cs->state);
141-
printf("cs->pcb = %p\n", cs->pcb);
142-
printf("cs->send_data = %p, len = %zu\n", cs->send_data, cs->send_data_len);
143128
mrb_state *mrb = cs->mrb;
144129
picorb_warn("TCPClient_poll_cb (timeout)\n");
145130
cs->state = NET_TCP_STATE_TIMEOUT;
@@ -151,9 +136,6 @@ TCPClient_err_cb(void *arg, err_t err)
151136
{
152137
if (!arg) return;
153138
tcp_connection_state *cs = (tcp_connection_state *)arg;
154-
printf("cs->state = %d\n", cs->state);
155-
printf("cs->pcb = %p\n", cs->pcb);
156-
printf("cs->send_data = %p, len = %zu\n", cs->send_data, cs->send_data_len);
157139
mrb_state *mrb = cs->mrb;
158140
picorb_warn("Error with: %d\n", err);
159141
cs->state = NET_TCP_STATE_ERROR;
@@ -189,6 +171,7 @@ TCPClient_new_tls_connection(mrb_state *mrb, const net_request_t *req, net_respo
189171
cs->pcb = altcp_tls_new(tls_config, IPADDR_TYPE_V4);
190172
if (!cs->pcb) {
191173
picorb_warn("altcp_tls_new failed\n");
174+
altcp_tls_free_config(tls_config);
192175
picorb_free(mrb, cs);
193176
return NULL;
194177
}
@@ -271,31 +254,35 @@ TCPClient_poll_impl(tcp_connection_state **pcs)
271254
return cs->state;
272255
}
273256

274-
void
257+
bool
275258
TCPClient_send(mrb_state *mrb, const net_request_t *req, net_response_t *res)
276259
{
260+
bool ret = false;
277261
ip_addr_t ip;
278262
ip4_addr_set_zero(&ip);
279263
Net_get_ip(req->host, &ip);
280-
if(!ip4_addr_isloopback(&ip)) {
264+
if (!ip4_addr_isloopback(&ip)) {
281265
char ip_str[16];
282266
ipaddr_ntoa_r(&ip, ip_str, 16);
283267
tcp_connection_state *cs = TCPClient_connect_impl(mrb, &ip, req, res);
284268
if (cs) {
285-
int max_wait = 1000;
269+
int max_wait = 200;
286270
while (TCPClient_poll_impl(&cs) && 0 < max_wait--) {
287271
// res->recv_data is ready after connection is complete
288272
Net_sleep_ms(100);
289273
}
290274
if (max_wait <= 0) {
291275
picorb_warn("TCPClient_send: timeout\n");
292-
picorb_free(mrb, cs->recv_data);
293276
} else {
294277
res->recv_data = cs->recv_data;
295278
res->recv_data_len = cs->recv_data_len;
296279
}
297-
TCPClient_close(cs);
280+
if (cs->state == NET_TCP_STATE_FINISHED) {
281+
ret = true;
282+
}
283+
TCPClient_close(cs);
298284
}
299285
}
286+
return ret;
300287
}
301288

‎mrbgems/picoruby-net/src/udp.c‎

Lines changed: 35 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
#define NET_UDP_STATE_SENDING 1
99
#define NET_UDP_STATE_WAITING 2
1010
#define NET_UDP_STATE_RECEIVED 3
11+
#define NET_UDP_STATE_FINISHED 4
1112
#define NET_UDP_STATE_ERROR 99
1213

1314
/* UDP connection struct */
@@ -37,28 +38,21 @@ UDPClient_recv_cb(void *arg, struct udp_pcb *pcb, struct pbuf *p, const ip_addr_
3738
char *tmpbuf = picorb_alloc(mrb, p->tot_len + 1);
3839
pbuf_copy_partial(p, tmpbuf, p->tot_len, 0);
3940
tmpbuf[p->tot_len] = '\0';
40-
if (cs->recv_data == NULL) {
41-
cs->recv_data = picorb_alloc(mrb, p->tot_len + 1);
42-
cs->recv_data_len = p->tot_len;
43-
memcpy(cs->recv_data, tmpbuf, p->tot_len);
44-
} else {
45-
char *new_recv_data = (char *)picorb_realloc(mrb, cs->recv_data, cs->recv_data_len + p->tot_len + 1);
46-
if (new_recv_data == NULL) {
47-
picorb_warn("Failed to reallocate memory for recv_data\n");
48-
picorb_free(mrb, tmpbuf);
49-
picorb_free(mrb, cs->recv_data);
50-
return;
51-
}
52-
cs->recv_data = new_recv_data;
53-
memcpy(cs->recv_data + cs->recv_data_len, tmpbuf, p->tot_len);
54-
cs->recv_data_len += p->tot_len;
41+
assert(cs->recv_data);
42+
char *new_recv_data = (char *)picorb_realloc(mrb, cs->recv_data, cs->recv_data_len + p->tot_len + 1);
43+
if (new_recv_data == NULL) {
44+
picorb_free(mrb, tmpbuf);
45+
cs->state = NET_UDP_STATE_ERROR;
46+
return;
5547
}
56-
picorb_free(cs->mrb, tmpbuf);
48+
cs->recv_data = new_recv_data;
49+
memcpy(cs->recv_data + cs->recv_data_len, tmpbuf, p->tot_len);
50+
cs->recv_data_len += p->tot_len;
51+
picorb_free(mrb, tmpbuf);
5752
cs->state = NET_UDP_STATE_RECEIVED;
5853
pbuf_free(p);
5954
} else {
6055
cs->state = NET_UDP_STATE_ERROR;
61-
picorb_warn("State changed to NET_UDP_STATE_ERROR\n");
6256
}
6357
}
6458

@@ -130,7 +124,7 @@ UDPClient_poll_impl(udp_connection_state **pcs)
130124
case NET_UDP_STATE_WAITING:
131125
break;
132126
case NET_UDP_STATE_RECEIVED:
133-
cs->state = NET_UDP_STATE_NONE;
127+
cs->state = NET_UDP_STATE_FINISHED;
134128
return 0;
135129
case NET_UDP_STATE_ERROR:
136130
mrb_state *mrb = cs->mrb;
@@ -140,41 +134,45 @@ UDPClient_poll_impl(udp_connection_state **pcs)
140134
return cs->state;
141135
}
142136

143-
void
137+
static void
138+
UDPClient_close(udp_connection_state *cs)
139+
{
140+
if (cs == NULL) return;
141+
mrb_state *mrb = cs->mrb;
142+
lwip_begin();
143+
udp_remove(cs->pcb);
144+
lwip_end();
145+
picorb_free(mrb, cs);
146+
}
147+
148+
bool
144149
UDPClient_send(mrb_state *mrb, const net_request_t *req, net_response_t *res)
145150
{
151+
bool ret = false;
146152
ip_addr_t ip;
147153
ip4_addr_set_zero(&ip);
148154
Net_get_ip(req->host, &ip);
149155

150156
udp_connection_state *cs = NULL;
151157

152-
if(!ip4_addr_isloopback(&ip)) {
158+
if (!ip4_addr_isloopback(&ip)) {
153159
cs = UDPClient_send_impl(mrb, &ip, req, res);
154-
if (cs == NULL) {
155-
picorb_warn("Failed to create UDP connection\n");
156-
} else {
160+
if (cs) {
157161
int max_wait = 50;
158162
while (UDPClient_poll_impl(&cs) && 0 < max_wait--) {
159163
Net_sleep_ms(100);
160164
}
161165
if (max_wait <= 0) {
162-
picorb_warn("Polling timeout reached\n");
163-
picorb_free(mrb, cs->recv_data);
164-
picorb_free(mrb, cs);
165-
return;
166+
picorb_warn("UDPClient_send: timeout\n");
167+
} else {
168+
res->recv_data = cs->recv_data;
169+
res->recv_data_len = cs->recv_data_len;
166170
}
167-
res->recv_data = cs->recv_data;
168-
res->recv_data_len = cs->recv_data_len;
169-
if (cs != NULL) {
170-
picorb_warn("Cleaning up cs\n");
171-
lwip_begin();
172-
udp_remove(cs->pcb);
173-
lwip_end();
174-
picorb_free(mrb, cs);
171+
if (cs->state == NET_UDP_STATE_FINISHED) {
172+
ret = true;
175173
}
174+
UDPClient_close(cs);
176175
}
177-
} else {
178-
picorb_warn("IP is loopback, not sending\n");
179176
}
177+
return ret;
180178
}

0 commit comments

Comments
 (0)