Re: iproute2: no error message when link up command fails.

[Andreas Henriksson <andreas@fatal.se>]

On ons, 2008-07-16 at 15:53 -0700, Stephen Hemminger wrote:
> The netlink message in question is marked as type ERROR but the errno
> encoded in the message is zero.
> 
> 		if (h->nlmsg_type == NLMSG_ERROR) {
> 			struct nlmsgerr *err = (struct nlmsgerr*)NLMSG_DATA(h);
> 			if (l < sizeof(struct nlmsgerr)) {
> 				fprintf(stderr, "ERROR truncated\n");
> 			} else {
> 				errno = -err->error;
> 				if (errno == 0) {
> 					if (answer)
> 						memcpy(answer, h, h->nlmsg_len);
> 					return 0;
> 				}
> 				perror("RTNETLINK answers");
> 			}
> 
> So the netlink library just treats as a successful return.
Why? This seems like a really bad idea to me, and none of the callers in
iproute benefits from this as far as I can see.

Just ripping out the errno == 0 special casing looks like and option to
me, unless anyone can find a reason for it.
(It'll give an error message and an error exit code! The message will be
strange, but lets blame the kernel for that cosmetic issue. Atleast the
user got some kind of notification.)

Moving the "return 0;" inside the "if (answer)" would be another
(atleast for iproutes callers of the library functions)...

> To me it looks like the problem is in the kernel sending back
> a NLMSG_ERROR with errno of zero. Some code path isn't setting
> it up properly.

None the less, it would be be good if the application wouldn't poop it's
pants when it can be avoided - broken kernel or not.


diff --git a/lib/libnetlink.c b/lib/libnetlink.c
index 5ae64f7..4413165 100644
--- a/lib/libnetlink.c
+++ b/lib/libnetlink.c
@@ -348,12 +348,7 @@ int rtnl_talk(struct rtnl_handle *rtnl, struct nlmsghdr *n, pid_t peer,
 					fprintf(stderr, "ERROR truncated\n");
 				} else {
 					errno = -err->error;
-					if (errno == 0) {
-						if (answer)
-							memcpy(answer, h, h->nlmsg_len);
-						return 0;
-					}
-					perror("RTNETLINK answers");
+					perror("RTNETLINK error");
 				}
 				return -1;
 			}


-- 
Regards,
Andreas Henriksson