Wget PASV parsing

[Denys Vlasenko]

On Friday 23 November 2007 07:37, Richard Kojedzinszky wrote:
> Dear busybox community,
>
> I've been using busybox for a while, and ran into the problem when using
> wget behind an openbsd gw, with nat and ftp-proxy. Without this patch wget
> connects to the wrong host, so the operation fails. This fixes that, but
> increases size by ~60 bytes in my environment. But maybe it is worth to
> apply for full functionality.

+               if (!(str = strrchr(buf, ')')))
+                       goto pasv_error;

For readability, I prefer to not place assignments
inside if(). It gains nothing (code is not smaller).

+               *str = 0;       // clear trailing '\0'

Comment is wrong. Again, for readability, use = '\0',
so that reader don't need to guess what type is it.

+               if (!(str = strrchr(buf, '(')))
+                       goto pasv_error;
+               p = ++str;
+               for (nn=0; nn<4; nn++) {
+                       if (!(str = strchr(str, ',')))
+                               goto pasv_error;
+                       *str++ = '.';
+               }
+               str[-1] = 0;

+               if (inet_aton(p, &lsa->sin.sin_addr) == 0)
+                       goto pasv_error;

This ruins IPv6 support.

+               if (!(str = strchr((p=str), ',')))
+                       goto pasv_error;

TWO assignments inside if()! Do you hate people who will read your code?

--
vda