Signed-off-by: Cong Wang <[email protected]>
---
include/net/inet_addr.h | 20 ++++++++++++++++++++
net/core/netpoll.c | 24 ++----------------------
2 files changed, 22 insertions(+), 22 deletions(-)
diff --git a/include/net/inet_addr.h b/include/net/inet_addr.h
index 66a16fe..1379287 100644
--- a/include/net/inet_addr.h
+++ b/include/net/inet_addr.h
@@ -4,6 +4,7 @@
#include <linux/in.h>
#include <linux/in6.h>
#include <linux/socket.h>
+#include <linux/inet.h>
#include <net/addrconf.h>
union inet_addr {
@@ -59,4 +60,23 @@ static inline bool inet_addr_multicast(const union inet_addr *ipa)
}
#endif
+static inline int inet_pton(const char *str, union inet_addr *addr)
+{
+ const char *end;
+
+ if (!strchr(str, ':') &&
+ in4_pton(str, -1, (void *)addr, -1, &end) > 0) {
+ if (!*end)
+ return 0;
+ }
+ if (in6_pton(str, -1, addr->sin6.sin6_addr.s6_addr, -1, &end) > 0) {
+#if IS_ENABLED(CONFIG_IPV6)
+ if (!*end)
+ return 1;
+#else
+ return -1;
+#endif
+ }
+ return -1;
+}
#endif
diff --git a/net/core/netpoll.c b/net/core/netpoll.c
index c300358..6631b5e 100644
--- a/net/core/netpoll.c
+++ b/net/core/netpoll.c
@@ -907,26 +907,6 @@ void netpoll_print_options(struct netpoll *np)
}
EXPORT_SYMBOL(netpoll_print_options);
-static int netpoll_parse_ip_addr(const char *str, union inet_addr *addr)
-{
- const char *end;
-
- if (!strchr(str, ':') &&
- in4_pton(str, -1, (void *)addr, -1, &end) > 0) {
- if (!*end)
- return 0;
- }
- if (in6_pton(str, -1, addr->sin6.sin6_addr.s6_addr, -1, &end) > 0) {
-#if IS_ENABLED(CONFIG_IPV6)
- if (!*end)
- return 1;
-#else
- return -1;
-#endif
- }
- return -1;
-}
-
int netpoll_parse_options(struct netpoll *np, char *opt)
{
char *cur=opt, *delim;
@@ -946,7 +926,7 @@ int netpoll_parse_options(struct netpoll *np, char *opt)
if ((delim = strchr(cur, '/')) == NULL)
goto parse_failed;
*delim = 0;
- ipv6 = netpoll_parse_ip_addr(cur, &np->local_ip);
+ ipv6 = inet_pton(cur, &np->local_ip);
if (ipv6 < 0)
goto parse_failed;
else
@@ -982,7 +962,7 @@ int netpoll_parse_options(struct netpoll *np, char *opt)
if ((delim = strchr(cur, '/')) == NULL)
goto parse_failed;
*delim = 0;
- ipv6 = netpoll_parse_ip_addr(cur, &np->remote_ip);
+ ipv6 = inet_pton(cur, &np->remote_ip);
if (ipv6 < 0)
goto parse_failed;
else if (np->ipv6 != (bool)ipv6)
--
1.7.7.6
Hello.
On 27-06-2013 10:43, Cong Wang wrote:
> Signed-off-by: Cong Wang <[email protected]>
> ---
> include/net/inet_addr.h | 20 ++++++++++++++++++++
> net/core/netpoll.c | 24 ++----------------------
> 2 files changed, 22 insertions(+), 22 deletions(-)
> diff --git a/include/net/inet_addr.h b/include/net/inet_addr.h
> index 66a16fe..1379287 100644
> --- a/include/net/inet_addr.h
> +++ b/include/net/inet_addr.h
> @@ -4,6 +4,7 @@
> #include <linux/in.h>
> #include <linux/in6.h>
> #include <linux/socket.h>
> +#include <linux/inet.h>
> #include <net/addrconf.h>
>
> union inet_addr {
> @@ -59,4 +60,23 @@ static inline bool inet_addr_multicast(const union inet_addr *ipa)
> }
> #endif
>
> +static inline int inet_pton(const char *str, union inet_addr *addr)
> +{
> + const char *end;
> +
> + if (!strchr(str, ':') &&
> + in4_pton(str, -1, (void *)addr, -1, &end) > 0) {
> + if (!*end)
> + return 0;
> + }
> + if (in6_pton(str, -1, addr->sin6.sin6_addr.s6_addr, -1, &end) > 0) {
> +#if IS_ENABLED(CONFIG_IPV6)
> + if (!*end)
How about:
if (IS_ENABLED(CONFIG_IPV6) && !*end)
> + return 1;
> +#else
> + return -1;
> +#endif
> + }
> + return -1;
> +}
> #endif
WBR, Sergei
On Thu, 2013-06-27 at 14:43 +0800, Cong Wang wrote:
> Signed-off-by: Cong Wang <[email protected]>
> ---
> include/net/inet_addr.h | 20 ++++++++++++++++++++
> net/core/netpoll.c | 24 ++----------------------
> 2 files changed, 22 insertions(+), 22 deletions(-)
>
> diff --git a/include/net/inet_addr.h b/include/net/inet_addr.h
> index 66a16fe..1379287 100644
> --- a/include/net/inet_addr.h
> +++ b/include/net/inet_addr.h
> @@ -4,6 +4,7 @@
> #include <linux/in.h>
> #include <linux/in6.h>
> #include <linux/socket.h>
> +#include <linux/inet.h>
> #include <net/addrconf.h>
>
> union inet_addr {
> @@ -59,4 +60,23 @@ static inline bool inet_addr_multicast(const union inet_addr *ipa)
> }
> #endif
>
> +static inline int inet_pton(const char *str, union inet_addr *addr)
> +{
> + const char *end;
> +
> + if (!strchr(str, ':') &&
> + in4_pton(str, -1, (void *)addr, -1, &end) > 0) {
> + if (!*end)
> + return 0;
> + }
> + if (in6_pton(str, -1, addr->sin6.sin6_addr.s6_addr, -1, &end) > 0) {
> +#if IS_ENABLED(CONFIG_IPV6)
> + if (!*end)
> + return 1;
> +#else
> + return -1;
> +#endif
> + }
> + return -1;
> +}
[...]
The return values for this are awful. I think the return value should
be 0 for success or -EINVAL on error.
The caller can then check addr->sa.sa_family if they want to know the
address family immediately.
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
On Thu, 27 Jun 2013 14:43:15 +0800
Cong Wang <[email protected]> wrote:
> Signed-off-by: Cong Wang <[email protected]>
> ---
> include/net/inet_addr.h | 20 ++++++++++++++++++++
> net/core/netpoll.c | 24 ++----------------------
> 2 files changed, 22 insertions(+), 22 deletions(-)
>
> diff --git a/include/net/inet_addr.h b/include/net/inet_addr.h
> index 66a16fe..1379287 100644
> --- a/include/net/inet_addr.h
> +++ b/include/net/inet_addr.h
> @@ -4,6 +4,7 @@
> #include <linux/in.h>
> #include <linux/in6.h>
> #include <linux/socket.h>
> +#include <linux/inet.h>
> #include <net/addrconf.h>
>
> union inet_addr {
> @@ -59,4 +60,23 @@ static inline bool inet_addr_multicast(const union inet_addr *ipa)
> }
> #endif
>
> +static inline int inet_pton(const char *str, union inet_addr *addr)
> +{
>
A couple of comments:
1. No reason for this to be inline
2. If function has same name as userspace it must have same arguments
and return value. Either:
a. rename it to kinet_pton or some other name
b. make it work the same.
On 06/27/2013 06:18 PM, Sergei Shtylyov wrote:
>> Signed-off-by: Cong Wang <[email protected]>
>> ---
>> include/net/inet_addr.h | 20 ++++++++++++++++++++
>> net/core/netpoll.c | 24 ++----------------------
>> 2 files changed, 22 insertions(+), 22 deletions(-)
>> diff --git a/include/net/inet_addr.h b/include/net/inet_addr.h
>> index 66a16fe..1379287 100644
>> --- a/include/net/inet_addr.h
>> +++ b/include/net/inet_addr.h
[...]
>> @@ -59,4 +60,23 @@ static inline bool inet_addr_multicast(const union
>> inet_addr *ipa)
>> }
>> #endif
>>
>> +static inline int inet_pton(const char *str, union inet_addr *addr)
>> +{
>> + const char *end;
>> +
>> + if (!strchr(str, ':') &&
>> + in4_pton(str, -1, (void *)addr, -1, &end) > 0) {
>> + if (!*end)
>> + return 0;
>> + }
>> + if (in6_pton(str, -1, addr->sin6.sin6_addr.s6_addr, -1, &end) > 0) {
>> +#if IS_ENABLED(CONFIG_IPV6)
>> + if (!*end)
> How about:
> if (IS_ENABLED(CONFIG_IPV6) && !*end)
>> + return 1;
>> +#else
Sorry, managed to miss #else... #if could be avoided anyway, tho
this could require an extra patch.
>> + return -1;
>> +#endif
>> + }
>> + return -1;
>> +}
>> #endif
WBR, Sergei
On Thu, 2013-06-27 at 16:32 +0100, Ben Hutchings wrote:
> The return values for this are awful. I think the return value should
> be 0 for success or -EINVAL on error.
>
> The caller can then check addr->sa.sa_family if they want to know the
> address family immediately.
Indeed, I will fix it.
Thanks!
On Thu, 2013-06-27 at 14:51 -0700, Stephen Hemminger wrote:
> >
> > +static inline int inet_pton(const char *str, union inet_addr *addr)
> > +{
> >
>
> A couple of comments:
> 1. No reason for this to be inline
Okay, I will move them into net/core/utils.c.
> 2. If function has same name as userspace it must have same arguments
> and return value. Either:
> a. rename it to kinet_pton or some other name
> b. make it work the same.
>
Makes sense too me, I will try your option b) first, if it is over-kill
I will fall back to option a).
Thanks!