[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: proxy support for wget retriever



* Erik Andersen 

| On Tue Jan 02, 2001 at 11:00:49AM +0100, Tollef Fog Heen wrote:
| > * Joey Hess 
| > 
| > | Tollef Fog Heen wrote:
| > | > 
| > | > I think the patch is pretty self-explanatory.  It adds proxy support
| > | > for wget, and additionally, it _requires_ the user to input hostname
| > | > and directory.
| > | > 
| > | > Could somebody review it and apply (I'm in the NM queue and don't have an
| > | > account on debian.org).
| > | 
| > | I've applied it with several modifications (the loops around GET
| > | commands made no sense).
| > 
| > Thanks - the while loops where there so that we would get hostname and
| > directory information.  Else, wget-retriever should check that it has
| > a hostname and directory before trying to call wget.  Just having wget
| > dying isn't a very nice thing to do, imho.  Especially when we are
| > using busybox wget which doesn't give any error messages.
| 
| If you can show me where it should give an error, I'll
| add it right in...

Imho, it should require to get a hostname, and a directory.  My
solution was to add loops around the debconf questions.  Except for
proxy, which isn't required.

And I just saw a small bug in Joey's code.  Patch:

===================================================================
RCS file: /cvs/debian-boot/debian-installer/retriever/wget/wget-retriever.c,v
retrieving revision 1.11
diff -c -u -r1.11 wget-retriever.c
cvs server: conflicting specifications of output style
--- wget-retriever.c    2001/01/01 03:13:09     1.11
+++ wget-retriever.c    2001/01/02 11:15:41
@@ -23,7 +23,7 @@
        debconf->command(debconf, "GET", DEBCONF_BASE "http/hostname", NULL);
        hostname=strdup(debconf->value);
        debconf->command(debconf, "GET", DEBCONF_BASE "http/directory", NULL);
-       directory=debconf->value;
+       directory=strdup(debconf->value);
        debconf->command(debconf, "GET", DEBCONF_BASE "http/proxy", NULL);
        if (strcmp(debconf->value,"") == 0)
                if (setenv("http_proxy", debconf->value, 1) == -1)

Also, the manpage for setenv is not clear whether the value is copied
or not.  If it is not, we should add a strdup around the
debconf->value in the setenv.  (To prevent problems if we add more
questions).
-- 

Tollef Fog Heen
Unix _IS_ user friendly... It's just selective about who its friends are.



Reply to: