Re: [PATCH v7 1/9] connect: document why we sometimes call get_port after get_host_and_port

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

 



On 05/22/2016 10:03 AM, Mike Hommey wrote:
On Sun, May 22, 2016 at 08:07:05AM +0200, Torsten Bögershausen wrote:
On 22.05.16 01:17, Mike Hommey wrote:
Signed-off-by: Mike Hommey <mh@xxxxxxxxxxxx>
---
  connect.c | 6 ++++++
  1 file changed, 6 insertions(+)

diff --git a/connect.c b/connect.c
index c53f3f1..caa2a3c 100644
--- a/connect.c
+++ b/connect.c
@@ -742,6 +742,12 @@ struct child_process *git_connect(int fd[2], const char *url,
  			transport_check_allowed("ssh");
  			get_host_and_port(&ssh_host, &port);
+ /* get_host_and_port may not return a port even when
+			 * there is one: In the [host:port]:path case,
+			 * get_host_and_port is called with "[host:port]" and
+			 * returns "host:port" and NULL.
+			 * In that specific case, we still need to split the
+			 * port. */
Is it worth to mention that this case is "still supported legacy" ?
If it's worth mentioning anywhere, it seems to me it would start with
urls.txt?

Mike

I don't know.
urls.txt is for Git users, and connect.c is for Git developers.
urls.txt does not mention that Git follows any RFC when parsing the
URLS', it doesn't claim to be 100% compliant.
Even if it makes sense to do so, as many user simply expect Git to accept
RFC compliant URL's, and it makes the development easier, if there is an already
written specification, that describes all the details.
The parser is not 100% RFC compliant, one example:
- old-style usgage like "git clone [host:222]:~/path/to/repo are supported
To easy the live for developers, it could make sense why the code is as it is,
simply to avoid that people around the world suddenly run into problems,
when they upgrade the Git version.
So if there is a comment in the code, it could help new developers to understand
things easier.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]