[pacman-dev] Fwd: [arch-dev-public] [signoff] pacman 3.2.0

[Xavier]

On Tue, Aug 5, 2008 at 1:50 AM, Xavier <shiningxc at gmail.com> wrote:
> The relevant part of the code is indeed the one you showed, but you
> have to look around it too:
>        for (beg = file + i; beg < end; beg = file + i + 1) {
>                while (*beg == '/')
>                        ++beg, ++i;
>                for (++i; file + i < end && file[i] != '/'; ++i)
>                        /* nothing */;
>                if(beg < end)
>                        e = _ftp_cmd(conn, "CWD %.*s", file + i - beg, beg);
>                if (e != FTP_FILE_ACTION_OK) {
>                        _ftp_seterr(e);
>                        return (-1);
>                }
>        }
>

<snip>

> It looks very weird to check for the e error here, while on the
> previous line, e was only set conditionally (only if beg < end).
>

Actually, here is how that part looks like in upstream code :
http://www.freebsd.org/cgi/cvsweb.cgi/src/lib/libfetch/ftp.c?rev=1.102
	for (beg = file + i; beg < end; beg = file + i + 1) {
		while (*beg == '/')
			++beg, ++i;
		for (++i; file + i < end && file[i] != '/'; ++i)
			/* nothing */ ;
		e = ftp_cmd(conn, "CWD %.*s", file + i - beg, beg);
		if (e != FTP_FILE_ACTION_OK) {
			ftp_seterr(e);
			return (-1);
		}
	}

So it is the same except that ftp_cmd is always run, and thus e always set here.
But well, running that with stupid urls with double slashes does not
seem to work either. Now it is the ftp_cmd call (CWD command) which
will fail.

However, the same patch I mentioned above, supposed to reduce latency,
also fixes the issue as a side effect.
http://www.freebsd.org/cgi/cvsweb.cgi/src/lib/libfetch/ftp.c.diff?r1=1.91.2.1;r2=1.91.2.2

Anyway, the important part was to fix pacman, and I already made a
patch for that.
I also wanted to try to figure out what was happening in libdownload,
and I at least partly managed to do that. And I think I found reasons
for trying to follow the upstream code more closely. Aaron seems to be
willing to do that, so everything is perfect :)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: dl.c
Type: text/x-csrc
Size: 2539 bytes
Desc: not available
URL: <http://archlinux.org/pipermail/pacman-dev/attachments/20080805/ba13e5c9/attachment.c>