ths-guest@haydn.debian.org wrote:
> Sanitize the ifdef TEST maze in ethtool-lite.c.
Is this patch tested? This seems very shady:
> if (edata.data)
> {
> -#ifdef TEST
> - printf("%s is connected.\n", iface);
> -#else
> di_info("%s is connected.\n", iface);
> return CONNECTED;
> -#endif
> }
> - else
> +
> {
You're left with a code block in braces where an else used to be, not a
compile error, but probably not what the author of that code intended
either.
We have exactly ** TWO CHANCES * to get netcfg right for the final release.
It can be uploaded tomorrow, and again the next day. The upload on
wednesday is scheduled to happen to get the translations from the string
freeze into the archive. This is not a very safe time to be making code
changes, especially if they are untested and potentially intorduce bugs,
and do not seem to be changes that are really required for release.
> if (ioctl (fd, ctl, &ifr) >= 0)
> {
> -#ifdef TEST
> - printf("%s is %sconnected. (MII)\n", iface,
> - ((data[3] & 0x0004) == 0) ? "dis" : "");
> -#else
> + int ret = !(data[3] & 0x0004);
> +
> di_info ("%s is %sconnected. (MII)\n", iface,
> - ((data[3] & 0x0004) == 0) ? "dis" : "");
> -
> - return (((data[3] & 0x004) == 0) + 1);
> -#endif
> - }
> - else
> - {
> -#ifdef TEST
> - fprintf(stderr, "MII ioctl failed\n");
> - return 1;
> -#else
> - di_warning("MII ioctl failed for %s\n", iface);
> - return UNKNOWN;
> -#endif
> + (ret) ? "dis" : "");
> +
> + return ret ? DISCONNECTED : CONNECTED;
This also seems to be a rather large change. It seems to me that it
removes the possibility of returning UNKNOWN if the ioctl fails.
--
see shy jo
Attachment:
signature.asc
Description: Digital signature