git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v6 1/9] connect: call get_host_and_port() earlier

From
Mike Hommey <mh@glandium.org>
Date
May 20, 2016, 22:28 UTC
Message-ID
<20160520222831.GA7752@glandium.org>
In-Reply-To
<xmqqwpmobcd4.fsf@gitster.mtv.corp.google.com>
On Fri, May 20, 2016 at 03:20:23PM -0700, Junio C Hamano wrote:
Show 33 quoted lines
> Mike Hommey <mh@glandium.org> writes:
> 
> >> Can never happen because?
> >> 
> >>   !*port means get_host_and_port() made the "port" pointer point at
> >>   a NUL byte.  That does not happen because the only case port is
> >>   moved by that function is to have it point at a byte after we
> >>   found ':', and the "port" string is a decimal integer whose value
> >>   is between 0 and 65535, so there is no way port points at an empty
> >>   string.
> >> 
> >> OK.
> >
> > Do you want me to add this to the commit message in a possible v7?
> 
> No.
> 
> I was merely thinking aloud to see if "in a case that never can
> happen" is sufficient decsription.  I think it is ;-)
> 
> >> This looks strange....
> > v3 of this series did remove this get_port(), and broke the
> > '[host:port]:path' syntax as a consequence. The reason this happens is
> > that get_host_and_port, in that case, is called with [host:port], sees
> > the square brackets, and searches the port *after* the closing bracket,
> > because the usual case where square brackets appear is ipv6 addresses,
> > which contain colons, and the brackets in that case are used to separate
> > the host and the port.
> >
> > In that case, get_host_and_port returns "host:port" and null.
> 
> Doesn't that indicate that this codepath deserves some in-code
> comment?

What would be the usual way you'd do this? separate patch? or just doing it in one of the patches that touches the surrounding code?

Mike
Previous: Junio C HamanoNext: Mike Hommey
Message 6 of 17 in “connect: various cleanups”
  1. 0/9 connect: various cleanupsMike Hommey, May 17, 2016
  2. 1/9 connect: call get_host_and_port() earlierMike Hommey, May 17, 2016
  3. Junio C HamanoMay 20, 2016
  4. Mike HommeyMay 20, 2016
  5. Junio C HamanoMay 20, 2016
  6. Mike HommeyMay 20, 2016
  7. 2/9 connect: only match the host with core.gitProxyMike Hommey, May 17, 2016
  8. Junio C HamanoMay 20, 2016
  9. Mike HommeyMay 20, 2016
  10. Junio C HamanoMay 20, 2016
  11. 3/9 connect: fill the host header in the git protocol with the host and port variablesMike Hommey, May 17, 2016
  12. 4/9 connect: make parse_connect_url() return separated host and portMike Hommey, May 17, 2016
  13. 5/9 connect: group CONNECT_DIAG_URL handling codeMike Hommey, May 17, 2016
  14. 6/9 connect: make parse_connect_url() return the user part of the url as a separate valueMike Hommey, May 17, 2016
  15. 7/9 connect: change the --diag-url output to separate user and hostMike Hommey, May 17, 2016
  16. 8/9 connect: actively reject git:// urls with a user partMike Hommey, May 17, 2016
  17. 9/9 connect: move ssh command line preparation to a separate functionMike Hommey, May 17, 2016

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.