[PATCH 0/2] iputils: minor ninfod and ping6 fixes

[PATCH 0/2] iputils: minor ninfod and ping6 fixes

[Jan Synacek]

Hello,

When calling ping6 with the flowlabel (e.g. `ping6 -F 123 ::1'), it exited with
an error. For some reason, the errno was set when it should not have been. Maybe
it shouldn't be checked at all, maybe just checking flowlabel for values below
zero would be enough. I wanted to be on the safer side so I left the errno check
in there.

Also, I fixed the rest of the unused variables in ninfod.

Jan Synacek (2):
  ninfod: Fix more unused variables.
  ping6: Fix -F switch.

 ninfod/ni_ifaddrs.c | 8 +-------
 ping6.c             | 3 ++-
 2 files changed, 3 insertions(+), 8 deletions(-)

-- 
1.8.0.1

[PATCH 1/2] ninfod: Fix more unused variables.

[Jan Synacek]

| ni_ifaddrs.c: In function ‘ni_ifaddrs’:
| ni_ifaddrs.c:389:14: warning: variable ‘nlm_scope’ set but not used [-Wunused-but-set-variable]
| ni_ifaddrs.c:353:13: warning: variable ‘ifflist’ set but not used [-Wunused-but-set-variable]
| ni_ifaddrs.c:322:6: warning: variable ‘result’ set but not used [-Wunused-but-set-variable]

Signed-off-by: Jan Synacek <jsynacek@redhat.com>
---
 ninfod/ni_ifaddrs.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/ninfod/ni_ifaddrs.c b/ninfod/ni_ifaddrs.c
index 4225a5a..0820497 100644
--- a/ninfod/ni_ifaddrs.c
+++ b/ninfod/ni_ifaddrs.c
@@ -319,7 +319,6 @@ int ni_ifaddrs(struct ni_ifaddrs **ifap, sa_family_t family)
 
 	pid_t pid = getpid();
 	int seq = 0;
-	int result;
 	int build;		/* 0 or 1 */
 
 /* ---------------------------------- */
@@ -350,7 +349,6 @@ int ni_ifaddrs(struct ni_ifaddrs **ifap, sa_family_t family)
 		struct ni_ifaddrs *ifl = NULL, *ifa = NULL;
 		struct nlmsghdr *nlh, *nlh0;
 		void *data = NULL, *xdata = NULL;
-		uint16_t *ifflist = NULL;
 #ifndef IFA_LOCAL
 		struct rtmaddr_ifamap ifamap;
 #endif
@@ -362,18 +360,15 @@ int ni_ifaddrs(struct ni_ifaddrs **ifap, sa_family_t family)
 				*ifap = ifa;
 			else {
 				free_data(data);
-				result = 0;
 				break;
 			}
 			if (data == NULL) {
 				free_data(data);
-				result = -1;
 				break;
 			}
 			ifl = NULL;
 			data += NLMSG_ALIGN(sizeof(struct ni_ifaddrs)) * icnt;
 			xdata = data + dlen;
-			ifflist = xdata + xlen;
 		}
 
 		for (nlm = nlmsg_list; nlm; nlm = nlm->nlm_next) {
@@ -386,7 +381,7 @@ int ni_ifaddrs(struct ni_ifaddrs **ifap, sa_family_t family)
 
 				size_t nlm_struct_size = 0;
 				sa_family_t nlm_family = 0;
-				uint32_t nlm_scope = 0, nlm_index = 0;
+				uint32_t nlm_index = 0;
 				unsigned int nlm_flags;
 				size_t rtasize;
 
@@ -405,7 +400,6 @@ int ni_ifaddrs(struct ni_ifaddrs **ifap, sa_family_t family)
 					ifam = (struct ifaddrmsg *) NLMSG_DATA(nlh);
 					nlm_struct_size = sizeof(*ifam);
 					nlm_family = ifam->ifa_family;
-					nlm_scope = ifam->ifa_scope;
 					nlm_index = ifam->ifa_index;
 					nlm_flags = ifam->ifa_flags;
 					if (family && nlm_family != family)
-- 
1.8.0.1

[PATCH 2/2] ping6: Fix -F switch.

[Jan Synacek]

Even when the flowlabel is set correctly, ping6 exits with a warning. For some
reason, the errno is set when it should not be.

Signed-off-by: Jan Synacek <jsynacek@redhat.com>
---
 ping6.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/ping6.c b/ping6.c
index 358a035..85d3782 100644
--- a/ping6.c
+++ b/ping6.c
@@ -725,7 +725,8 @@ int main(int argc, char *argv[])
 		switch(ch) {
 		case 'F':
 			flowlabel = hextoui(optarg);
-			if (errno || (flowlabel & ~IPV6_FLOWINFO_FLOWLABEL)) {
+			if ((flowlabel < 0 && errno) ||
+			    (flowlabel & ~IPV6_FLOWINFO_FLOWLABEL)) {
 				fprintf(stderr, "ping: Invalid flowinfo %s\n", optarg);
 				exit(2);
 			}
-- 
1.8.0.1

Re: [PATCH 2/2] ping6: Fix -F switch.

[Jan Synacek]

On 12/07/2012 09:25 AM, Jan Synacek wrote:
> Even when the flowlabel is set correctly, ping6 exits with a warning. For some
> reason, the errno is set when it should not be.
> 
> Signed-off-by: Jan Synacek <jsynacek@redhat.com>
> ---
>  ping6.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/ping6.c b/ping6.c
> index 358a035..85d3782 100644
> --- a/ping6.c
> +++ b/ping6.c
> @@ -725,7 +725,8 @@ int main(int argc, char *argv[])
>  		switch(ch) {
>  		case 'F':
>  			flowlabel = hextoui(optarg);
> -			if (errno || (flowlabel & ~IPV6_FLOWINFO_FLOWLABEL)) {
> +			if ((flowlabel < 0 && errno) ||
> +			    (flowlabel & ~IPV6_FLOWINFO_FLOWLABEL)) {
>  				fprintf(stderr, "ping: Invalid flowinfo %s\n", optarg);
>  				exit(2);
>  			}
> 

This was well meant, but it doesn't work, because flowlabel is declared as
__u32, therefore it can never be negative. I'm going to post a fixed version soon.

-- 
Jan Synacek
Software Engineer, BaseOS team Brno, Red Hat

Re: [PATCH 2/2 fixed] ping6: Fix -F switch.

[Jan Synacek]

Even when the flowlabel is set correctly, ping6 exits with a warning. The errno
should be checked only if the previous call returned a negative value. In this
case, there is no need to check errno, checking for a negative value is enough.

Signed-off-by: Jan Synacek <jsynacek@redhat.com>
---
 ping6.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/ping6.c b/ping6.c
index 358a035..94a24b0 100644
--- a/ping6.c
+++ b/ping6.c
@@ -724,8 +724,9 @@ int main(int argc, char *argv[])
 	while ((ch = getopt(argc, argv, COMMON_OPTSTR "F:N:")) != EOF) {
 		switch(ch) {
 		case 'F':
-			flowlabel = hextoui(optarg);
-			if (errno || (flowlabel & ~IPV6_FLOWINFO_FLOWLABEL)) {
+			err = hextoui(optarg);
+			flowlabel = (__u32)err;
+			if (err < 0 || (flowlabel & ~IPV6_FLOWINFO_FLOWLABEL)) {
 				fprintf(stderr, "ping: Invalid flowinfo %s\n", optarg);
 				exit(2);
 			}
-- 
1.8.0.1

Re: [PATCH 0/2] iputils: minor ninfod and ping6 fixes

[YOSHIFUJI Hideaki]

Jan Synacek wrote:

> When calling ping6 with the flowlabel (e.g. `ping6 -F 123 ::1'), it exited with
> an error. For some reason, the errno was set when it should not have been. Maybe
> it shouldn't be checked at all, maybe just checking flowlabel for values below
> zero would be enough. I wanted to be on the safer side so I left the errno check
> in there.
> 
> Also, I fixed the rest of the unused variables in ninfod.
> 
> Jan Synacek (2):
>   ninfod: Fix more unused variables.
>   ping6: Fix -F switch.
> 
>  ninfod/ni_ifaddrs.c | 8 +-------
>  ping6.c             | 3 ++-
>  2 files changed, 3 insertions(+), 8 deletions(-)
> 

Fixes committed. Thank you!

--yoshfuji