git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Johan Herland <johan@herland.net>
Cc: Junio C Hamano <gitster@pobox.com>,
	Shawn Pearce <spearce@spearce.org>,
	git@vger.kernel.org, Johannes Sixt <j6t@kdbg.org>
Subject: [PATCH 2/3] connect: let callers know if connection is a socket
Date: Mon, 16 May 2011 02:52:11 -0400	[thread overview]
Message-ID: <20110516065211.GB19078@sigill.intra.peff.net> (raw)
In-Reply-To: <20110516063944.GB25731@sigill.intra.peff.net>

They might care because they want to do a half-duplex close.
With pipes, that means simply closing the output descriptor;
with a socket, you must actually call shutdown.

Instead of exposing the magic no_fork child_process struct,
let's encapsulate the test in a function.

Signed-off-by: Jeff King <peff@peff.net>
---
An more object-oriented refactoring would be something like:

  struct git_connection {
    struct child_process child;
    int in;
    int out;
  };

  void git_connection_read_fd(struct git_connection *c)
  {
          return c->in;
  }

  void git_connect_write_fd(struct git_connect *c)
  {
          return c->child.pid ? c->out : c->in;
  }

  void git_connection_half_duplex_close(struct git_connection *c)
  {
          if (!c->child.pid)
                  shutdown(c->in, SHUT_WR);
          else
                  close(c->out);
  }

but the idea that a git connection is defined by two file descriptors
runs throughout the code (in fact, we don't even explicitly do the
half-duplex close in the pipe case; we hand the descriptor off to the
pack-objects run-command, which takes ownership). So trying to be fancy
and abstracted is not worth it in this case.

 cache.h   |    1 +
 connect.c |    7 ++++++-
 2 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/cache.h b/cache.h
index bf468e5..724aad4 100644
--- a/cache.h
+++ b/cache.h
@@ -865,6 +865,7 @@ extern struct ref *find_ref_by_name(const struct ref *list, const char *name);
 #define CONNECT_VERBOSE       (1u << 0)
 extern struct child_process *git_connect(int fd[2], const char *url, const char *prog, int flags);
 extern int finish_connect(struct child_process *conn);
+extern int git_connection_is_socket(struct child_process *conn);
 extern int path_match(const char *path, int nr, char **match);
 struct extra_have_objects {
 	int nr, alloc;
diff --git a/connect.c b/connect.c
index 86ad150..20a008d 100644
--- a/connect.c
+++ b/connect.c
@@ -634,10 +634,15 @@ struct child_process *git_connect(int fd[2], const char *url_orig,
 	return conn;
 }
 
+int git_connection_is_socket(struct child_process *conn)
+{
+	return conn == &no_fork;
+}
+
 int finish_connect(struct child_process *conn)
 {
 	int code;
-	if (!conn || conn == &no_fork)
+	if (!conn || git_connection_is_socket(conn))
 		return 0;
 
 	code = finish_command(conn);
-- 
1.7.5.1.13.g0ca09.dirty

  parent reply	other threads:[~2011-05-16  6:52 UTC|newest]

Thread overview: 53+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-05-13 16:54 [PATCH 2/2] receive-pack: Add receive.denyObjectLimit to refuse push with too many objects Johan Herland
2011-05-13 17:09 ` Junio C Hamano
2011-05-14  1:43   ` Johan Herland
2011-05-14  2:03     ` [PATCHv2 2/2] receive-pack: Add receive.objectCountLimit " Johan Herland
2011-05-14  2:30       ` Shawn Pearce
2011-05-14 13:17         ` Johan Herland
2011-05-14 22:17           ` Shawn Pearce
2011-05-15 17:42             ` Johan Herland
2011-05-15 21:37               ` [PATCHv3 0/9] Push limits Johan Herland
2011-05-15 21:37                 ` [PATCHv3 1/9] Update technical docs to reflect side-band-64k capability in receive-pack Johan Herland
2011-05-15 21:37                 ` [PATCHv3 2/9] send-pack: Attempt to retrieve remote status even if pack-objects fails Johan Herland
2011-05-16  4:07                   ` Jeff King
2011-05-16  6:13                     ` Jeff King
2011-05-16  6:39                       ` Jeff King
2011-05-16  6:46                         ` [PATCH 1/3] connect: treat generic proxy processes like ssh processes Jeff King
2011-05-16 19:57                           ` Johannes Sixt
2011-05-16 23:12                             ` Junio C Hamano
2011-05-17  5:54                             ` Jeff King
2011-05-17 20:14                               ` Johannes Sixt
2011-05-18  8:57                                 ` Jeff King
2011-05-16  6:52                         ` Jeff King [this message]
2011-05-16  6:52                         ` [PATCH 3/3] send-pack: avoid deadlock on git:// push with failed pack-objects Jeff King
2011-05-16 20:02                           ` Johannes Sixt
2011-05-17  5:56                             ` Jeff King
2011-05-18 20:24                               ` [PATCH] Windows: add a wrapper for the shutdown() system call Johannes Sixt
2011-05-15 21:37                 ` [PATCHv3 3/9] pack-objects: Allow --max-pack-size to be used together with --stdout Johan Herland
2011-05-15 22:06                   ` Shawn Pearce
2011-05-16  1:39                     ` Johan Herland
2011-05-16  6:12                   ` Junio C Hamano
2011-05-16  9:27                     ` Johan Herland
2011-05-15 21:37                 ` [PATCHv3 4/9] pack-objects: Teach new option --max-object-count, similar to --max-pack-size Johan Herland
2011-05-15 22:07                   ` Shawn Pearce
2011-05-15 22:31                     ` Johan Herland
2011-05-15 23:48                       ` Shawn Pearce
2011-05-16  6:25                     ` Junio C Hamano
2011-05-16  9:49                       ` Johan Herland
2011-05-15 21:37                 ` [PATCHv3 5/9] pack-objects: Teach new option --max-commit-count, limiting #commits in pack Johan Herland
2011-05-15 21:37                 ` [PATCHv3 6/9] receive-pack: Prepare for addition of the new 'limit-*' family of capabilities Johan Herland
2011-05-16  6:50                   ` Junio C Hamano
2011-05-16  9:53                     ` Johan Herland
2011-05-16 22:02                     ` Sverre Rabbelier
2011-05-16 22:07                       ` Junio C Hamano
2011-05-16 22:09                         ` Sverre Rabbelier
2011-05-16 22:12                           ` Junio C Hamano
2011-05-16 22:16                             ` Sverre Rabbelier
2011-05-15 21:37                 ` [PATCHv3 7/9] send-pack/receive-pack: Allow server to refuse pushes with too many objects Johan Herland
2011-05-15 21:37                 ` [PATCHv3 8/9] send-pack/receive-pack: Allow server to refuse pushing too large packs Johan Herland
2011-05-15 21:37                 ` [PATCHv3 9/9] send-pack/receive-pack: Allow server to refuse pushes with too many commits Johan Herland
2011-05-15 21:52                 ` [PATCHv3 0/9] Push limits Ævar Arnfjörð Bjarmason
2011-05-14 17:50         ` [PATCHv2 2/2] receive-pack: Add receive.objectCountLimit to refuse push with too many objects Junio C Hamano
2011-05-14 22:27           ` Shawn Pearce
2011-05-13 18:20 ` [PATCH 2/2] receive-pack: Add receive.denyObjectLimit " Johannes Sixt
2011-05-14  1:49   ` Johan Herland

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20110516065211.GB19078@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=j6t@kdbg.org \
    --cc=johan@herland.net \
    --cc=spearce@spearce.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).