@matheusbsbs / matheus-bagliano / commits / df1ed03a6f

remote-ext: simplify git pkt-line generation

We format a pkt-line into a heap buffer, which requires manual computation of the required size, and uses some bare sprintf calls. We could use a strbuf instead, which would take care of the computation for us. But it's even easier still to use packet_write(). Besides handling the formatting and writing for us, it fixes two things: 1. Our manual max-size check used 0xFFFF, while technically LARGE_PACKET_MAX is slightly smaller than this. 2. Our packet will now be output as part of GIT_TRACE_PACKET debugging. Unfortunately packet_write() does not let us build up the buffer progressively, so we do have to repeat ourselves a little depending on the "vhost" setting, but the end result is still far more readable than the original. Since there were no tests covering this feature at all, we'll add a few into t5802. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Sep 24, 2015 at 17:07 UTC df1ed03a6fabf58b63b26950c71d36eff4aa25e8

2 files changed +33 -29

builtin/remote-ext.c

+5 -29

@@ -1,6 +1,7 @@

1 #include "builtin.h"

2 #include "transport.h"

3 #include "run-command.h"

4 +#include "pkt-line.h"

6 /*

7 * URL syntax:

@@ -142,36 +143,11 @@ static const char **parse_argv(const char *arg, const char *service)

143 static void send_git_request(int stdin_fd, const char *serv, const char *repo,

144 const char *vhost)

145 {

145 - size_t bufferspace;

146 - size_t wpos = 0;

147 - char *buffer;

148 -

149 - /*

150 - * Request needs 12 bytes extra if there is vhost (xxxx \0host=\0) and

151 - * 6 bytes extra (xxxx \0) if there is no vhost.

152 - */

153 - if (vhost)

154 - bufferspace = strlen(serv) + strlen(repo) + strlen(vhost) + 12;

146 + if (!vhost)

147 + packet_write(stdin_fd, "%s %s%c", serv, repo, 0);

148 else

156 - bufferspace = strlen(serv) + strlen(repo) + 6;

157 -

158 - if (bufferspace > 0xFFFF)

159 - die("Request too large to send");

160 - buffer = xmalloc(bufferspace);

161 -

162 - /* Make the packet. */

163 - wpos = sprintf(buffer, "%04x%s %s%c", (unsigned)bufferspace,

164 - serv, repo, 0);

165 -

166 - /* Add vhost if any. */

167 - if (vhost)

168 - sprintf(buffer + wpos, "host=%s%c", vhost, 0);

169 -

170 - /* Send the request */

171 - if (write_in_full(stdin_fd, buffer, bufferspace) < 0)

172 - die_errno("Failed to send request");

173 -

174 - free(buffer);

149 + packet_write(stdin_fd, "%s %s%chost=%s%c", serv, repo, 0,

150 + vhost, 0);

151 }

152

153 static int run_child(const char *arg, const char *service)

t/t5802-connect-helper.sh

+28

@@ -69,4 +69,32 @@ test_expect_success 'update backfilled tag without primary transfer' '

69 test_cmp expect actual

70 '

72 +

73 +test_expect_success 'set up fake git-daemon' '

74 + mkdir remote &&

75 + git init --bare remote/one.git &&

76 + mkdir remote/host &&

77 + git init --bare remote/host/two.git &&

78 + write_script fake-daemon <<-\EOF &&

79 + git daemon --inetd \

80 + --informative-errors \

81 + --export-all \

82 + --base-path="$TRASH_DIRECTORY/remote" \

83 + --interpolated-path="$TRASH_DIRECTORY/remote/%H%D" \

84 + "$TRASH_DIRECTORY/remote"

85 + EOF

86 + export TRASH_DIRECTORY &&

87 + PATH=$TRASH_DIRECTORY:$PATH

88 +'

89 +

90 +test_expect_success 'ext command can connect to git daemon (no vhost)' '

91 + rm -rf dst &&

92 + git clone "ext::fake-daemon %G/one.git" dst

93 +'

94 +

95 +test_expect_success 'ext command can connect to git daemon (vhost)' '

96 + rm -rf dst &&

97 + git clone "ext::fake-daemon %G/two.git %Vhost" dst

98 +'

99 +

100 test_done