Re: [PATCH v8 0/9] connect: various cleanups
On Sat, May 28, 2016 at 10:17:19AM +0200, Torsten Bögershausen wrote: > On 28.05.16 07:33, Mike Hommey wrote: > > On Sat, May 28, 2016 at 07:02:01AM +0200, Torsten Bögershausen wrote: > >> On 27.05.16 23:59, Mike Hommey wrote: > >>> On Fri, May 27, 2016 at 04:24:20PM +0200, Torsten Bögershausen wrote: > On 27.05.16 04:27, Mike Hommey wrote: > > Changes from v7: > > - Fixed comments. > > > > Mike Hommey (9): > >> All in all, a great improvement. > >> 2 things left. > >> - The comment about [] is now stale, isn't it ? > > No, it's still valid at this point, that's what the 2nd argument to > > host_end (0) does. > > > > Mike > > The protocol (specific) code doesn't do this anymore, > because that is now common to all protocols. > > >/* > * Don't do destructive transforms now, the > * '[]' unwrapping is done in get_host_and_port() > */ > I'm not following what you're saying. The code explicitly calls host_end so that it doesn't remove the square brackets from the string it's passed. That's what the comment says, and that's what happens. Mike -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v8 0/9] connect: various cleanups
On 28.05.16 07:33, Mike Hommey wrote: > On Sat, May 28, 2016 at 07:02:01AM +0200, Torsten Bögershausen wrote: >> On 27.05.16 23:59, Mike Hommey wrote: >>> On Fri, May 27, 2016 at 04:24:20PM +0200, Torsten Bögershausen wrote: On 27.05.16 04:27, Mike Hommey wrote: > Changes from v7: > - Fixed comments. > > Mike Hommey (9): >> All in all, a great improvement. >> 2 things left. >> - The comment about [] is now stale, isn't it ? > No, it's still valid at this point, that's what the 2nd argument to > host_end (0) does. > > Mike The protocol (specific) code doesn't do this anymore, because that is now common to all protocols. /* * Don't do destructive transforms now, the * '[]' unwrapping is done in get_host_and_port() */ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v8 0/9] connect: various cleanups
On Sat, May 28, 2016 at 07:02:01AM +0200, Torsten Bögershausen wrote: > On 27.05.16 23:59, Mike Hommey wrote: > > On Fri, May 27, 2016 at 04:24:20PM +0200, Torsten Bögershausen wrote: > >> On 27.05.16 04:27, Mike Hommey wrote: > >>> Changes from v7: > >>> - Fixed comments. > >>> > >>> Mike Hommey (9): > All in all, a great improvement. > 2 things left. > - The comment about [] is now stale, isn't it ? No, it's still valid at this point, that's what the 2nd argument to host_end (0) does. Mike -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v8 0/9] connect: various cleanups
On 27.05.16 23:59, Mike Hommey wrote: > On Fri, May 27, 2016 at 04:24:20PM +0200, Torsten Bögershausen wrote: >> On 27.05.16 04:27, Mike Hommey wrote: >>> Changes from v7: >>> - Fixed comments. >>> >>> Mike Hommey (9): All in all, a great improvement. 2 things left. - The comment about [] is now stale, isn't it ? - The legacy support should only be active for ssh, and not be used by other schemes. diff --git a/connect.c b/connect.c index 076ae09..79b8dae 100644 --- a/connect.c +++ b/connect.c @@ -618,10 +618,6 @@ static enum protocol parse_connect_url(const char *url_orig, char **ret_user, } } - /* -* Don't do destructive transforms as protocol code does -* '[]' unwrapping in get_host_and_port() -*/ end = host_end(&host, 0); if (protocol == PROTO_LOCAL) @@ -673,7 +669,7 @@ static enum protocol parse_connect_url(const char *url_orig, char **ret_user, * "host:port" and NULL. * To support this undocumented legacy we still need to split the port. */ - if (!port) + if (!port && protocol == PROTO_SSH) port = get_port(host); *ret_user = user ? xstrdup(user) : NULL; -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v8 0/9] connect: various cleanups
On Fri, May 27, 2016 at 04:24:20PM +0200, Torsten Bögershausen wrote: > On 27.05.16 04:27, Mike Hommey wrote: > > Changes from v7: > > - Fixed comments. > > > > Mike Hommey (9): > > connect: document why we sometimes call get_port after > > get_host_and_port > > connect: call get_host_and_port() earlier > > connect: re-derive a host:port string from the separate host and port > > variables > > connect: make parse_connect_url() return separated host and port > > connect: group CONNECT_DIAG_URL handling code > > connect: make parse_connect_url() return the user part of the url as a > > separate value > > connect: change the --diag-url output to separate user and host > > connect: actively reject git:// urls with a user part > > connect: move ssh command line preparation to a separate function > > > > connect.c | 235 > > +- > > t/t5500-fetch-pack.sh | 42 ++--- > > 2 files changed, 170 insertions(+), 107 deletions(-) > > > Is the whole series somewhere on a public repo ? Is it now :) > No major remarks so far, if possible, I would like > to have a look at the resulting connect.c https://github.com/glandium/git/blob/connect/connect.c Mike -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v8 0/9] connect: various cleanups
On 27.05.16 04:27, Mike Hommey wrote: > Changes from v7: > - Fixed comments. > > Mike Hommey (9): > connect: document why we sometimes call get_port after > get_host_and_port > connect: call get_host_and_port() earlier > connect: re-derive a host:port string from the separate host and port > variables > connect: make parse_connect_url() return separated host and port > connect: group CONNECT_DIAG_URL handling code > connect: make parse_connect_url() return the user part of the url as a > separate value > connect: change the --diag-url output to separate user and host > connect: actively reject git:// urls with a user part > connect: move ssh command line preparation to a separate function > > connect.c | 235 > +- > t/t5500-fetch-pack.sh | 42 ++--- > 2 files changed, 170 insertions(+), 107 deletions(-) > Is the whole series somewhere on a public repo ? No major remarks so far, if possible, I would like to have a look at the resulting connect.c -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
