Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pkg/paho-mqtt: add support for gnrc #17004

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Prev Previous commit
Next Next commit
fixup! pkg/paho-mqtt: add support for gnrc
  • Loading branch information
vera committed Dec 3, 2021
commit 3e93d65eb7d0b64444ad7696034d64e2844877fa
25 changes: 18 additions & 7 deletions pkg/paho-mqtt/contrib/riot_iface.c
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,9 @@ static int mqtt_read(struct Network *n, unsigned char *buf, int len,

#if defined(MODULE_LWIP)
/* As LWIP doesn't support packet reading byte per byte and
* PAHO MQTT reads like that to decode it on the fly,
* we read TSRB_MAX_SIZE at once and keep them in a ring buffer.
*/
* PAHO MQTT reads like that to decode it on the fly,
* we read TSRB_MAX_SIZE at once and keep them in a ring buffer.
*/
_buf = _temp_buf;
_len = TSRB_MAX_SIZE;
_timeout = 0;
Expand Down Expand Up @@ -134,13 +134,24 @@ int NetworkConnect(Network *n, char *addr_ip, int port)
ipv4_addr_from_str((ipv4_addr_t *)&remote.addr, _local_ip)) {
remote.port = port;
}
else if (IS_USED(MODULE_IPV6_ADDR)) {
/* ipvN_addr_from_str modifies input buffer */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure about that? The char pointer is declared as const and briefly skimming the code doesn't reveal any bug that results in disregarding the constness.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think you are right and I also wondered about this but I kept the comment unchanged since I thought it is not directly relevant for my changes and I didn't want to overcomplicate this PR. I will take a look at this again.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't see a reason not to use ipvN_addr_from_str() directly. (And I have also found another place where this is already done: tests/lwip/ip.c:116 and tests/lwip/ip.c:130)

I have removed the comment and the strncpy.

strncpy(_local_ip, addr_ip, sizeof(_local_ip));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm personally not a fan of strncpy, as it does not guarantee to zero-terminate strings. It should not be needed if ipvN_addr_from_str() is used directly (which I believe it can), but if I'm wrong about that: How about something like this

size_t addr_ip_len = strlen(addr_ip);
if (addr_ip_len + 1 > sizeof(_local_ip)) {
    return -EINVAL;
}
memcpy(_local_ip, addr_ip, addr_ip_len);
_local_ip[addr_ip_len] = [0];


#if defined(MODULE_GNRC)
char *iface = ipv6_addr_split_iface(_local_ip);
if ((!iface) && (gnrc_netif_numof() == 1)) {
remote.netif = gnrc_netif_iter(NULL)->pid;
}
else if (iface) {
remote.netif = atoi(iface);
}
#endif

/* ipvN_addr_from_str modifies input buffer */
strncpy(_local_ip, addr_ip, sizeof(_local_ip));
if (IS_USED(MODULE_IPV6_ADDR) && (remote.port == 0) &&
ipv6_addr_from_str((ipv6_addr_t *)&remote.addr, _local_ip)) {
if (ipv6_addr_from_str((ipv6_addr_t *)&remote.addr, _local_ip)) {
remote.port = port;
remote.family = AF_INET6;
}
}

if (remote.port == 0) {
Expand Down