You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2022/08/01 09:25:01 UTC
[GitHub] [incubator-nuttx] PetervdPerk-NXP opened a new pull request, #6751: Net thread-safe ether_ntoa_r & inet_ntoa_r
PetervdPerk-NXP opened a new pull request, #6751:
URL: https://github.com/apache/incubator-nuttx/pull/6751
## Summary
Normal ntoa use a static buffer which consumes memory and isn't thread-safe the _r variant passes the char* to write the data too.
## Impact
Saves around 32 bytes by not statically allocated the buffers
Minimal ntoa calls replaced by ntoa_r calls, apps need to be updated as well to save on memory usage.
## Testing
```
Before
peter@NXL04520:~/nuttx/nuttx$ arm-none-eabi-size nuttx
text data bss dec hex filename
159348 784 23920 184052 2cef4 nuttx
peter@NXL04520:~/nuttx/nuttx$
After
peter@NXL04520:~/nuttx/nuttx$ arm-none-eabi-size nuttx
text data bss dec hex filename
159356 784 23888 184028 2cedc nuttx
```
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on pull request #6751: Net thread-safe ether_ntoa_r & inet_ntoa_r
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on PR #6751:
URL: https://github.com/apache/incubator-nuttx/pull/6751#issuecomment-1202319654
Please squash into a single commit
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] hartmannathan commented on a diff in pull request #6751: Net thread-safe ether_ntoa_r & inet_ntoa_r
Posted by GitBox <gi...@apache.org>.
hartmannathan commented on code in PR #6751:
URL: https://github.com/apache/incubator-nuttx/pull/6751#discussion_r934869568
##########
drivers/wireless/gs2200m.c:
##########
@@ -2018,7 +2018,10 @@ static enum pkt_type_e gs2200m_send_bulk(FAR struct gs2200m_dev_s *dev,
}
else
{
- wlinfo("** addr=%s port=%d\n", inet_ntoa(msg->addr.sin_addr),
+ char inetaddr[INET_ADDRSTRLEN + 2];
Review Comment:
> why +2 is needed?
This is why it is better not to put hard-coded constant literals in the code, unless there is a comment that explains the rationale behind that number.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6751: Net thread-safe ether_ntoa_r & inet_ntoa_r
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6751:
URL: https://github.com/apache/incubator-nuttx/pull/6751#discussion_r934605876
##########
drivers/wireless/gs2200m.c:
##########
@@ -2018,7 +2018,10 @@ static enum pkt_type_e gs2200m_send_bulk(FAR struct gs2200m_dev_s *dev,
}
else
{
- wlinfo("** addr=%s port=%d\n", inet_ntoa(msg->addr.sin_addr),
+ char inetaddr[INET_ADDRSTRLEN + 2];
Review Comment:
why +2 is needed?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] PetervdPerk-NXP commented on a diff in pull request #6751: Net thread-safe ether_ntoa_r & inet_ntoa_r
Posted by GitBox <gi...@apache.org>.
PetervdPerk-NXP commented on code in PR #6751:
URL: https://github.com/apache/incubator-nuttx/pull/6751#discussion_r934614801
##########
drivers/wireless/gs2200m.c:
##########
@@ -2018,7 +2018,10 @@ static enum pkt_type_e gs2200m_send_bulk(FAR struct gs2200m_dev_s *dev,
}
else
{
- wlinfo("** addr=%s port=%d\n", inet_ntoa(msg->addr.sin_addr),
+ char inetaddr[INET_ADDRSTRLEN + 2];
Review Comment:
No clue actually, came from inetntoa static allocation.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6751: Net thread-safe ether_ntoa_r & inet_ntoa_r
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6751:
URL: https://github.com/apache/incubator-nuttx/pull/6751#discussion_r934531233
##########
include/arpa/inet.h:
##########
@@ -50,6 +50,7 @@ in_addr_t inet_addr(FAR const char *cp);
in_addr_t inet_network(FAR const char *cp);
FAR char *inet_ntoa(struct in_addr in);
+FAR char *inet_ntoa_r(struct in_addr in, FAR char *buf);
Review Comment:
Yes. That should be fine!
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6751: Net thread-safe ether_ntoa_r & inet_ntoa_r
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6751:
URL: https://github.com/apache/incubator-nuttx/pull/6751#discussion_r934605089
##########
libs/libc/net/lib_etherntoa_r.c:
##########
@@ -0,0 +1,52 @@
+/****************************************************************************
+ * libs/libc/net/lib_etherntoa_r.c
+ *
+ * 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.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+#include <stdio.h>
+
+#include <net/ethernet.h>
+#include <netinet/ether.h>
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: ether_ntoa_r
+ *
+ * Description:
+ * The ether_ntoa_r() function converts the Ethernet host address addr
+ * given in network byte order to a string in standard
+ * hex-digits-and-colons notation.
+ *
+ ****************************************************************************/
+
+FAR char *ether_ntoa_r(FAR const struct ether_addr *addr, FAR char *buf)
+{
+ sprintf(buf, "%02x:%02x:%02x:%02x:%02x:%02x",
Review Comment:
`ether_ntoa_r` knows nothing about memory that is represented by `buf`, so using of `snprintf` will not give any benefits here
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6751: Net thread-safe ether_ntoa_r & inet_ntoa_r
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6751:
URL: https://github.com/apache/incubator-nuttx/pull/6751#discussion_r934699558
##########
drivers/wireless/gs2200m.c:
##########
@@ -2018,7 +2018,10 @@ static enum pkt_type_e gs2200m_send_bulk(FAR struct gs2200m_dev_s *dev,
}
else
{
- wlinfo("** addr=%s port=%d\n", inet_ntoa(msg->addr.sin_addr),
+ char inetaddr[INET_ADDRSTRLEN + 2];
Review Comment:
No, nnn.nnn.nnn.nnn has 15(4*4 - 1) char. 16bytes is enough to contain nnn.nnn.nnn.nnn plus \0
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6751: Net thread-safe ether_ntoa_r & inet_ntoa_r
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6751:
URL: https://github.com/apache/incubator-nuttx/pull/6751#discussion_r934726485
##########
drivers/wireless/gs2200m.c:
##########
@@ -2018,7 +2018,10 @@ static enum pkt_type_e gs2200m_send_bulk(FAR struct gs2200m_dev_s *dev,
}
else
{
- wlinfo("** addr=%s port=%d\n", inet_ntoa(msg->addr.sin_addr),
+ char inetaddr[INET_ADDRSTRLEN + 2];
Review Comment:
Yes. No +1 or +2 is needed
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] PetervdPerk-NXP commented on a diff in pull request #6751: Net thread-safe ether_ntoa_r & inet_ntoa_r
Posted by GitBox <gi...@apache.org>.
PetervdPerk-NXP commented on code in PR #6751:
URL: https://github.com/apache/incubator-nuttx/pull/6751#discussion_r934437072
##########
include/arpa/inet.h:
##########
@@ -50,6 +50,7 @@ in_addr_t inet_addr(FAR const char *cp);
in_addr_t inet_network(FAR const char *cp);
FAR char *inet_ntoa(struct in_addr in);
+FAR char *inet_ntoa_r(struct in_addr in, FAR char *buf);
Review Comment:
Ah I see, still many small variants though.
Do you agree on the following?
`char * inet_ntoa_r(struct in_addr in, FAR char *buf, size_t bufflen);`
I prefer to use `size_t` just like `strncpy`
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] PetervdPerk-NXP commented on a diff in pull request #6751: Net thread-safe ether_ntoa_r & inet_ntoa_r
Posted by GitBox <gi...@apache.org>.
PetervdPerk-NXP commented on code in PR #6751:
URL: https://github.com/apache/incubator-nuttx/pull/6751#discussion_r934682870
##########
drivers/wireless/gs2200m.c:
##########
@@ -2018,7 +2018,10 @@ static enum pkt_type_e gs2200m_send_bulk(FAR struct gs2200m_dev_s *dev,
}
else
{
- wlinfo("** addr=%s port=%d\n", inet_ntoa(msg->addr.sin_addr),
+ char inetaddr[INET_ADDRSTRLEN + 2];
Review Comment:
Well I would add +1 for the zero terminator, question is why it used to be +2?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] PetervdPerk-NXP commented on a diff in pull request #6751: Net thread-safe ether_ntoa_r & inet_ntoa_r
Posted by GitBox <gi...@apache.org>.
PetervdPerk-NXP commented on code in PR #6751:
URL: https://github.com/apache/incubator-nuttx/pull/6751#discussion_r934437072
##########
include/arpa/inet.h:
##########
@@ -50,6 +50,7 @@ in_addr_t inet_addr(FAR const char *cp);
in_addr_t inet_network(FAR const char *cp);
FAR char *inet_ntoa(struct in_addr in);
+FAR char *inet_ntoa_r(struct in_addr in, FAR char *buf);
Review Comment:
Ah I see, still many small variants though.
Do you agree on the following?
`char * inet_ntoa_r(struct in_addr in, FAR char *buf, size_t size);`
I prefer to use `size_t` just like `strncpy`
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6751: Net thread-safe ether_ntoa_r & inet_ntoa_r
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6751:
URL: https://github.com/apache/incubator-nuttx/pull/6751#discussion_r935296866
##########
net/procfs/netdev_statistics.c:
##########
@@ -194,9 +194,13 @@ static int netprocfs_linklayer(FAR struct netprocfs_file_s *netfile)
#if defined(CONFIG_NET_ETHERNET) || defined(CONFIG_DRIVERS_IEEE80211)
case NET_LL_ETHERNET:
case NET_LL_IEEE80211:
- len += snprintf(&netfile->line[len], NET_LINELEN - len,
+ {
+ char hwaddr[20];
+ len += snprintf(&netfile->line[len], NET_LINELEN - len,
"%s\tLink encap:Ethernet HWaddr %s",
- dev->d_ifname, ether_ntoa(&dev->d_mac.ether));
+ dev->d_ifname,
+ ether_ntoa_r(&dev->d_mac.ether, hwaddr));
Review Comment:
```suggestion
dev->d_ifname,
ether_ntoa_r(&dev->d_mac.ether, hwaddr));
```
##########
net/procfs/netdev_statistics.c:
##########
@@ -194,9 +194,13 @@ static int netprocfs_linklayer(FAR struct netprocfs_file_s *netfile)
#if defined(CONFIG_NET_ETHERNET) || defined(CONFIG_DRIVERS_IEEE80211)
case NET_LL_ETHERNET:
case NET_LL_IEEE80211:
- len += snprintf(&netfile->line[len], NET_LINELEN - len,
+ {
+ char hwaddr[20];
+ len += snprintf(&netfile->line[len], NET_LINELEN - len,
"%s\tLink encap:Ethernet HWaddr %s",
Review Comment:
```suggestion
"%s\tLink encap:Ethernet HWaddr %s",
```
##########
drivers/wireless/gs2200m.c:
##########
@@ -2027,7 +2030,8 @@ static enum pkt_type_e gs2200m_send_bulk(FAR struct gs2200m_dev_s *dev,
snprintf(cmd, sizeof(cmd), "%cY%c%s:%d:%s",
ASCII_ESC, msg->cid,
- inet_ntoa(msg->addr.sin_addr), NTOHS(msg->addr.sin_port),
+ inet_ntoa_r(msg->addr.sin_addr, inetaddr, sizeof(inetaddr)),
+ NTOHS(msg->addr.sin_port),
Review Comment:
```suggestion
inet_ntoa_r(msg->addr.sin_addr, inetaddr, sizeof(inetaddr)),
NTOHS(msg->addr.sin_port),
```
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] PetervdPerk-NXP commented on a diff in pull request #6751: Net thread-safe ether_ntoa_r & inet_ntoa_r
Posted by GitBox <gi...@apache.org>.
PetervdPerk-NXP commented on code in PR #6751:
URL: https://github.com/apache/incubator-nuttx/pull/6751#discussion_r934399519
##########
include/arpa/inet.h:
##########
@@ -50,6 +50,7 @@ in_addr_t inet_addr(FAR const char *cp);
in_addr_t inet_network(FAR const char *cp);
FAR char *inet_ntoa(struct in_addr in);
+FAR char *inet_ntoa_r(struct in_addr in, FAR char *buf);
Review Comment:
Since `inet_ntoa_r` isn't really specified in POSIX I tried to make it close as the GNU `ether_ntoa_r` variant.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6751: Net thread-safe ether_ntoa_r & inet_ntoa_r
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6751:
URL: https://github.com/apache/incubator-nuttx/pull/6751#discussion_r934622511
##########
drivers/wireless/gs2200m.c:
##########
@@ -2018,7 +2018,10 @@ static enum pkt_type_e gs2200m_send_bulk(FAR struct gs2200m_dev_s *dev,
}
else
{
- wlinfo("** addr=%s port=%d\n", inet_ntoa(msg->addr.sin_addr),
+ char inetaddr[INET_ADDRSTRLEN + 2];
Review Comment:
Ok. I will check. Let's keep for now
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6751: Net thread-safe ether_ntoa_r & inet_ntoa_r
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6751:
URL: https://github.com/apache/incubator-nuttx/pull/6751#discussion_r934623178
##########
drivers/wireless/gs2200m.c:
##########
@@ -2027,7 +2030,8 @@ static enum pkt_type_e gs2200m_send_bulk(FAR struct gs2200m_dev_s *dev,
snprintf(cmd, sizeof(cmd), "%cY%c%s:%d:%s",
ASCII_ESC, msg->cid,
- inet_ntoa(msg->addr.sin_addr), NTOHS(msg->addr.sin_port),
+ inet_ntoa_r(msg->addr.sin_addr, inetaddr, sizeof(inetaddr)),
+ NTOHS(msg->addr.sin_port),
Review Comment:
```suggestion
inet_ntoa_r(msg->addr.sin_addr, inetaddr, sizeof(inetaddr)),
NTOHS(msg->addr.sin_port),
```
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6751: Net thread-safe ether_ntoa_r & inet_ntoa_r
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6751:
URL: https://github.com/apache/incubator-nuttx/pull/6751#discussion_r934676777
##########
drivers/wireless/gs2200m.c:
##########
@@ -2018,7 +2018,10 @@ static enum pkt_type_e gs2200m_send_bulk(FAR struct gs2200m_dev_s *dev,
}
else
{
- wlinfo("** addr=%s port=%d\n", inet_ntoa(msg->addr.sin_addr),
+ char inetaddr[INET_ADDRSTRLEN + 2];
Review Comment:
Here is the definition:
```
/* Sizes of addresses (per OpenGroup.org) */
#define INET_ADDRSTRLEN 16 /* nnn.nnn.nnn.nnn */
#define INET6_ADDRSTRLEN 46 /* xxxx:xxxx:xxxx:xxxx:xxxx:xxxx:xxxx:xxxx */
```
it's safe to define char inetaddr[INET_ADDRSTRLEN].
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6751: Net thread-safe ether_ntoa_r & inet_ntoa_r
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6751:
URL: https://github.com/apache/incubator-nuttx/pull/6751#discussion_r934427251
##########
include/arpa/inet.h:
##########
@@ -50,6 +50,7 @@ in_addr_t inet_addr(FAR const char *cp);
in_addr_t inet_network(FAR const char *cp);
FAR char *inet_ntoa(struct in_addr in);
+FAR char *inet_ntoa_r(struct in_addr in, FAR char *buf);
Review Comment:
Yeah, but
- http://www.qnx.com/developers/docs/7.0.0/index.html#com.qnx.doc.neutrino.lib_ref/topic/i/inet_ntoa_r.html
- https://www.ibm.com/docs/en/i/7.3?topic=ssw_ibm_i_73/apis/tsinntoa.htm
- https://nxmnpg.lemoda.net/3/inet_ntoa_r
- https://github.com/espressif/esp-idf/blob/master/examples/protocols/sockets/tcp_server/main/tcp_server.c
- https://asf.microchip.com/docs/latest/sam4s/html/inet_8h.html
- https://www.unix.com/man-page/freebsd/3/inet_ntoa_r/
I would stick to some more common implementation even if it not closer to GNU `ether_ntoa_r` variant.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6751: Net thread-safe ether_ntoa_r & inet_ntoa_r
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6751:
URL: https://github.com/apache/incubator-nuttx/pull/6751#discussion_r934429213
##########
libs/libc/net/lib_etherntoa_r.c:
##########
@@ -0,0 +1,52 @@
+/****************************************************************************
+ * libs/libc/net/lib_etherntoa_r.c
+ *
+ * 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.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+#include <stdio.h>
+
+#include <net/ethernet.h>
+#include <netinet/ether.h>
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: ether_ntoa_r
+ *
+ * Description:
+ * The ether_ntoa_r() function converts the Ethernet host address addr
+ * given in network byte order to a string in standard
+ * hex-digits-and-colons notation.
+ *
+ ****************************************************************************/
+
+FAR char *ether_ntoa_r(FAR const struct ether_addr *addr, FAR char *buf)
Review Comment:
Maybe better to define `ether_ntoa_r` in `libs/libc/net/lib_etherntoa.c` and change `ether_ntoa` to
```
FAR char *ether_ntoa(FAR const struct ether_addr *addr)
{
static char buffer[20];
return ether_ntoa_r(addr, buffer);
}
```
?
##########
libs/libc/net/lib_inetntoa_r.c:
##########
@@ -0,0 +1,55 @@
+/****************************************************************************
+ * libs/libc/net/lib_inetntoa_r.c
Review Comment:
same as for `ether_ntoa_r`
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] hartmannathan commented on a diff in pull request #6751: Net thread-safe ether_ntoa_r & inet_ntoa_r
Posted by GitBox <gi...@apache.org>.
hartmannathan commented on code in PR #6751:
URL: https://github.com/apache/incubator-nuttx/pull/6751#discussion_r934566557
##########
libs/libc/net/lib_etherntoa_r.c:
##########
@@ -0,0 +1,52 @@
+/****************************************************************************
+ * libs/libc/net/lib_etherntoa_r.c
+ *
+ * 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.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+#include <stdio.h>
+
+#include <net/ethernet.h>
+#include <netinet/ether.h>
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: ether_ntoa_r
+ *
+ * Description:
+ * The ether_ntoa_r() function converts the Ethernet host address addr
+ * given in network byte order to a string in standard
+ * hex-digits-and-colons notation.
+ *
+ ****************************************************************************/
+
+FAR char *ether_ntoa_r(FAR const struct ether_addr *addr, FAR char *buf)
+{
+ sprintf(buf, "%02x:%02x:%02x:%02x:%02x:%02x",
Review Comment:
Use snprintf() here instead?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 merged pull request #6751: Net thread-safe ether_ntoa_r & inet_ntoa_r
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged PR #6751:
URL: https://github.com/apache/incubator-nuttx/pull/6751
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6751: Net thread-safe ether_ntoa_r & inet_ntoa_r
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6751:
URL: https://github.com/apache/incubator-nuttx/pull/6751#discussion_r934619676
##########
libs/libc/net/lib_etherntoa_r.c:
##########
@@ -0,0 +1,52 @@
+/****************************************************************************
+ * libs/libc/net/lib_etherntoa_r.c
Review Comment:
Maybe we can follow `libs/libc/net/lib_etheraton.c` example and implement both non-`_r` and `_r` interfaces in a single file?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6751: Net thread-safe ether_ntoa_r & inet_ntoa_r
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6751:
URL: https://github.com/apache/incubator-nuttx/pull/6751#discussion_r934349909
##########
drivers/wireless/gs2200m.c:
##########
@@ -2018,7 +2018,9 @@ static enum pkt_type_e gs2200m_send_bulk(FAR struct gs2200m_dev_s *dev,
}
else
{
- wlinfo("** addr=%s port=%d\n", inet_ntoa(msg->addr.sin_addr),
+ char inetaddr[INET_ADDRSTRLEN + 2];
+
+ wlinfo("** addr=%s port=%d\n", inet_ntoa(msg->addr.sin_addr, inetaddr),
Review Comment:
```suggestion
wlinfo("** addr=%s port=%d\n", inet_ntoa_r(msg->addr.sin_addr, inetaddr),
```
##########
include/arpa/inet.h:
##########
@@ -50,6 +50,7 @@ in_addr_t inet_addr(FAR const char *cp);
in_addr_t inet_network(FAR const char *cp);
FAR char *inet_ntoa(struct in_addr in);
+FAR char *inet_ntoa_r(struct in_addr in, FAR char *buf);
Review Comment:
most of the variants that I lookup in the Internet give me `char * inet_ntoa_r(struct in_addr in, char *buf, socklen_t size);`, so why `size` is not added in this variant?
##########
drivers/wireless/gs2200m.c:
##########
@@ -2027,7 +2029,8 @@ static enum pkt_type_e gs2200m_send_bulk(FAR struct gs2200m_dev_s *dev,
snprintf(cmd, sizeof(cmd), "%cY%c%s:%d:%s",
ASCII_ESC, msg->cid,
- inet_ntoa(msg->addr.sin_addr), NTOHS(msg->addr.sin_port),
+ inet_ntoa(msg->addr.sin_addr, inetaddr),
Review Comment:
```suggestion
inet_ntoa_r(msg->addr.sin_addr, inetaddr),
```
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6751: Net thread-safe ether_ntoa_r & inet_ntoa_r
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6751:
URL: https://github.com/apache/incubator-nuttx/pull/6751#discussion_r934427251
##########
include/arpa/inet.h:
##########
@@ -50,6 +50,7 @@ in_addr_t inet_addr(FAR const char *cp);
in_addr_t inet_network(FAR const char *cp);
FAR char *inet_ntoa(struct in_addr in);
+FAR char *inet_ntoa_r(struct in_addr in, FAR char *buf);
Review Comment:
Yeah, but
http://www.qnx.com/developers/docs/7.0.0/index.html#com.qnx.doc.neutrino.lib_ref/topic/i/inet_ntoa_r.html
https://www.ibm.com/docs/en/i/7.3?topic=ssw_ibm_i_73/apis/tsinntoa.htm
https://nxmnpg.lemoda.net/3/inet_ntoa_r
https://github.com/espressif/esp-idf/blob/master/examples/protocols/sockets/tcp_server/main/tcp_server.c
https://asf.microchip.com/docs/latest/sam4s/html/inet_8h.html
https://www.unix.com/man-page/freebsd/3/inet_ntoa_r/
I would stick to some more common implementation even if it not closer to GNU `ether_ntoa_r` variant.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org