Hello Daniel,

Please find two attachments, one showing the error on test 1, and the other
containing the next revision of the patch.

The comments here relate to your comments below. I hope the relationship
is clear. The line-numbers are line-numbers in the new version of the patch.

o In memdebug.c about line 52, I've changed mem to a long double, so it
will be correctly aligned if the enclosing structure is correctly
aligned. This stopped my SIGBUS and it's the way it's done in the example
in K&R.

o In include/curl/curl.h at lines 348 and 518, CURLOPT_NETRC gets a public
enum. Undefined values have the same effect as OPTIONAL, which I have
documented in the man page.

o In url.c at line 1314, I think I was taking the view that this setting
is for safety, that bits is part of the output, and the real work finding
out what the user/password is and whether there is one happens at about
line 1862 and should not be duplicated.

o In url.c, I'm not testing anywhere for illegal values of
CURLOPT_NETRC. If I should, where would you recommend putting it? This
relates to undefined values and OPTIONAL above.

o In url.c at line 1797, I've added a comment that I think explains what
I'm doing. I've looked at the RFC and think we can save a special case,
because the port number will always be digits-only and the remainder of an
IPV6 address will always contain a ']'. Please tell me if I've made it too
simple to work.

o In main.c, at too many lines to mention, there is now a long argument
for the optional case. I think having the optional case be harder than the
required case is the right choice, because -n with a user/password in the
URL didn't work before.

o I now do override netrc's as you suggested, so I don't need to scribble
the real one. The test harness can no longer write ~/... files. Override
netrc's are enabled by MALLOCDEBUG because _NETRC_DEBUG defines a main()
that is incompatible with rebuilding curl.

o The test for skipping netrc tests is really ugly because ${$x} only
works for global variables. Eval strikes me as evil, because it can have
side-effects in the implementation of the test harness. I would be
interested in a less evil solution for the 'requires' attribute in the test
harness.

o My five tests pass.

o I've updated the man page and MANUAL. I'm having a formatting problem
with the man page, on the words "Only machine name ...", which is attached
and for which I would welcome advice.

Regards,
James Cone.

At 10:41 3/04/2002 +0200, Daniel Stenberg wrote:
>On Wed, 3 Apr 2002, J. Cone wrote:
>
> > Please find enclosed a patch which:
> > - is released under the MIT licence with the permission of Ms K Couper
> > at eServGlobal in Wellington
> > - is against curl 7.9.5
>
>*thanks* (any particular reason why Ms K Couper isn't posting this himself?)
>
>I appreciate the amount of comments in the patch and its general usability,
>but I have a few remarks on the patch, as follows here. Please don't take
>this as anything else than me being careful with what goes into the main
>sources. Feel free to tell me I'm wrong, if you think I am.
>
>o Instead of setting the internal 'set.use_netrc' variable to -1, 1 or 0, I
> would prefer to have it use defines/enums with descriptive names.
> It makes it less confusing in the code where it is later used.
>
>o The removal of the line (url.c around 1320):
> conn->bits.user_passwd = data->set.userpwd?1:0;
>
> is that *really* thought through and motivated? Can you please elaborate on
> why this isn't needed for cases where CURLOPT_NETRC is not used.
>
>o The comment that says "There could be a colon in the user name / password
> part" is wrong. Colons are "reserved" in that part of the URL and may
> only be used as delimiters. If user names or passwords contain colons, they
> must be URL encoded. (RFC2396 section 3.2.2)
>
> Thus, using strrchr() instead of strchr() to find the colon isn't needed
> but of course does no harm.
>
>o The src/main.c patch that introduces a different meaning of -n when used
> twice is not that nice imho. The curl tool has no other option that gets
> such a different meaning when used twice. I really can't see the need for
> this either, this new netrc stuff is mostly for library uses when you want
> to override what the URL contains, while a user of a command line tool will
> most often control both the options and the URL.
>
> If the new functionality really is wanted from the command line, I suggest
> we use a new (long) option instead and keep the old functionality as it is
> today.
>
> > - WARNING WARNING DANGER WILL ROBINSON
> > - the test cases 130 .. 133 (the new ones) will destroy your .netrc
> > - given that they test .netrc reading, I don't know how to avoid that
>
>*Ugha*. While I of course appreciate the provided test cases, we really
>cannot have this behavior there.
>
>A suggested approach to fix this: we introduce a "hack" in libcurl that when
>compiled with --enable-debug changes the behavior of Curl_parsenetrc()
>slightly, and if a certain environment variable is set, it will identify
>which file to use instead of the one that would normally be used.
>
>Then, in a similar style which we check for memory debugging in the test
>suite, we check for netrc debugging as well and we only run the netrc test
>cases if curl is compiled to work with them.
>
>I'm open for other ideas. We MUST NOT modify a single file outside of the
>tests directory.
>
> > Test-case 1 (HTTP) fails with an error I don't understand, which is
> > attached.
>
>I didn't find any error text attached. Did you forget to include that or did
>I miss it?
>
>--
> Daniel Stenberg -- curl groks URLs -- http://curl.haxx.se/

_______________________________________________________________

Have big pipes? SourceForge.net is looking for download mirrors. We supply
the hardware. You get the recognition. Email Us: bandwidth_at_sourceforge.net