0e/8d7870912fa5a255aea67aef9463550a835ab1

   1 Return-Path: <sojkam1@fel.cvut.cz>\r
   2 X-Original-To: notmuch@notmuchmail.org\r
   3 Delivered-To: notmuch@notmuchmail.org\r
   4 Received: from localhost (localhost [127.0.0.1])\r
   5         by olra.theworths.org (Postfix) with ESMTP id 385B9431FAF\r
   6         for <notmuch@notmuchmail.org>; Thu, 20 Sep 2012 09:38:04 -0700 (PDT)\r
   7 X-Virus-Scanned: Debian amavisd-new at olra.theworths.org\r
   8 X-Spam-Flag: NO\r
   9 X-Spam-Score: -2.3\r
  10 X-Spam-Level: \r
  11 X-Spam-Status: No, score=-2.3 tagged_above=-999 required=5\r
  12         tests=[RCVD_IN_DNSWL_MED=-2.3] autolearn=disabled\r
  13 Received: from olra.theworths.org ([127.0.0.1])\r
  14         by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024)\r
  15         with ESMTP id OADjm0cGuJWs for <notmuch@notmuchmail.org>;\r
  16         Thu, 20 Sep 2012 09:38:03 -0700 (PDT)\r
  17 Received: from max.feld.cvut.cz (max.feld.cvut.cz [147.32.192.36])\r
  18         by olra.theworths.org (Postfix) with ESMTP id C1FEA431FAE\r
  19         for <notmuch@notmuchmail.org>; Thu, 20 Sep 2012 09:38:02 -0700 (PDT)\r
  20 Received: from localhost (unknown [192.168.200.4])\r
  21         by max.feld.cvut.cz (Postfix) with ESMTP id CBE9F19F33B0;\r
  22         Thu, 20 Sep 2012 18:37:57 +0200 (CEST)\r
  23 X-Virus-Scanned: IMAP AMAVIS\r
  24 Received: from max.feld.cvut.cz ([192.168.200.1])\r
  25         by localhost (styx.feld.cvut.cz [192.168.200.4]) (amavisd-new,\r
  26         port 10044)\r
  27         with ESMTP id X4nqNXQtRn0q; Thu, 20 Sep 2012 18:37:55 +0200 (CEST)\r
  28 Received: from imap.feld.cvut.cz (imap.feld.cvut.cz [147.32.192.34])\r
  29         by max.feld.cvut.cz (Postfix) with ESMTP id DE76E19F3392;\r
  30         Thu, 20 Sep 2012 18:37:55 +0200 (CEST)\r
  31 Received: from steelpick.2x.cz (note-sojka.felk.cvut.cz [147.32.86.30])\r
  32         (Authenticated sender: sojkam1)\r
  33         by imap.feld.cvut.cz (Postfix) with ESMTPSA id BC95E660904;\r
  34         Thu, 20 Sep 2012 18:37:52 +0200 (CEST)\r
  35 Received: from wsh by steelpick.2x.cz with local (Exim 4.80)\r
  36         (envelope-from <sojkam1@fel.cvut.cz>)\r
  37         id 1TEjl9-0003E9-Ub; Thu, 20 Sep 2012 18:37:51 +0200\r
  38 From: Michal Sojka <sojkam1@fel.cvut.cz>\r
  39 To: Tomi Ollila <tomi.ollila@iki.fi>, notmuch@notmuchmail.org\r
  40 Subject: Re: [PATCH V3 1/2] test/smtp-dummy: add --background option and\r
  41         functionality\r
  42 In-Reply-To: <1347978182-8771-1-git-send-email-tomi.ollila@iki.fi>\r
  43 References: <1347978182-8771-1-git-send-email-tomi.ollila@iki.fi>\r
  44 User-Agent: Notmuch/0.14+23~g9d68aca (http://notmuchmail.org) Emacs/24.2.1\r
  45         (x86_64-pc-linux-gnu)\r
  46 Date: Thu, 20 Sep 2012 18:37:51 +0200\r
  47 Message-ID: <87zk4k4pls.fsf@steelpick.2x.cz>\r
  48 MIME-Version: 1.0\r
  49 Content-Type: text/plain\r
  50 Cc: Tomi Ollila <too@iki.fi>\r
  51 X-BeenThere: notmuch@notmuchmail.org\r
  52 X-Mailman-Version: 2.1.13\r
  53 Precedence: list\r
  54 List-Id: "Use and development of the notmuch mail system."\r
  55         <notmuch.notmuchmail.org>\r
  56 List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
  57         <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
  58 List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
  59 List-Post: <mailto:notmuch@notmuchmail.org>\r
  60 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
  61 List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
  62         <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
  63 X-List-Received-Date: Thu, 20 Sep 2012 16:38:04 -0000\r
  64 \r
  65 Hi Tomi,\r
  66 \r
  67 On Tue, Sep 18 2012, Tomi Ollila wrote:\r
  68 > From: Tomi Ollila <too@iki.fi>\r
  69 >\r
  70 > When shell executes background process using '&' the scheduling of\r
  71 > that new process is arbitrary. It could be that smtp-dummy doesn't\r
  72 > get execution time to listen() it's server socket until some other\r
  73 > process attempts to connect() to it. The --background option in\r
  74 > smtp-dummy makes it to go background *after* it started to listen\r
  75 > its server socket.\r
  76 >\r
  77 > When --background option is used, the line "smtp_dummy_pid='<pid>'"\r
  78 > is printed to stdout from where shell can eval it.\r
  79 > ---\r
  80 >\r
  81 > This is v3 of id:"1323766883-17607-1-git-send-email-tomi.ollila@iki.fi"\r
  82 >\r
  83 > addressing (some) Dmitry's comments.\r
  84 >\r
  85 >  test/smtp-dummy.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-\r
  86 >  1 file changed, 54 insertions(+), 1 deletion(-)\r
  87 >\r
  88 > diff --git a/test/smtp-dummy.c b/test/smtp-dummy.c\r
  89 > index 86d4316..bb13668 100644\r
  90 > --- a/test/smtp-dummy.c\r
  91 > +++ b/test/smtp-dummy.c\r
  92 > @@ -119,6 +119,7 @@ do_smtp_to_file (FILE *peer, FILE *output)\r
  93 >  int\r
  94 >  main (int argc, char *argv[])\r
  95 >  {\r
  96 > +     const char * progname;\r
  97 >       char *output_filename;\r
  98 >       FILE *peer_file, *output;\r
  99 >       int sock, peer, err;\r
 100 > @@ -126,9 +127,31 @@ main (int argc, char *argv[])\r
 101 >       struct hostent *hostinfo;\r
 102 >       socklen_t peer_addr_len;\r
 103 >       int reuse;\r
 104 > +     int background;\r
 105 > +\r
 106 > +     progname = argv[0];\r
 107 > +\r
 108 > +     background = 0;\r
 109 > +     for (; argc >= 2; argc--, argv++) {\r
 110 > +             if (argv[1][0] != '-')\r
 111 \r
 112 This is ugly, but all right. I do not think we need to have high\r
 113 standards for this piece of code :)\r
 114 \r
 115 > +                     break;\r
 116 > +             if (strcmp (argv[1], "--") == 0) {\r
 117 > +                     argc--;\r
 118 > +                     argv++;\r
 119 > +                     break;\r
 120 > +             }\r
 121 > +             if (strcmp (argv[1], "--background") == 0) {\r
 122 > +                     background = 1;\r
 123 > +                     continue;\r
 124 > +             }\r
 125 > +             fprintf(stderr, "%s: unregognized option '%s'\n",\r
 126 > +                     progname, argv[1]);\r
 127 > +             return 1;\r
 128 > +     }\r
 129 >\r
 130 >       if (argc != 2) {\r
 131 > -             fprintf (stderr, "Usage: %s <output-file>\n", argv[0]);\r
 132 > +             fprintf (stderr,\r
 133 > +                      "Usage: %s [--background] <output-file>\n", progname);\r
 134 >               return 1;\r
 135 >       }\r
 136 >\r
 137 > @@ -181,6 +204,36 @@ main (int argc, char *argv[])\r
 138 >               return 1;\r
 139 >       }\r
 140 >\r
 141 > +     if (background) {\r
 142 > +             int pid = fork ();\r
 143 > +             if (pid > 0) {\r
 144 > +                     printf ("smtp_dummy_pid='%d'\n", pid);\r
 145 > +                     fflush (stdout);\r
 146 > +                     close (sock);\r
 147 > +                     return 0;\r
 148 > +             }\r
 149 > +             if (pid < 0) {\r
 150 > +                     fprintf (stderr, "Error: fork() failed: %s\n",\r
 151 > +                              strerror (errno));\r
 152 > +                     close (sock);\r
 153 > +                     return 1;\r
 154 > +             }\r
 155 > +             /* Reached if pid == 0 (the child process). */\r
 156 > +             /* Close stdout so that the one interested in pid value will\r
 157 > +                also get EOF. */\r
 158 > +             close (STDOUT_FILENO);\r
 159 > +             /* dup2() will re-reserve fd of stdout (1) (opportunistically),\r
 160 > +                in case fd of stderr (2) is open. If that was not open we\r
 161 > +                don't care fd of stdout (1) either. */\r
 162 > +             dup2 (STDERR_FILENO, STDOUT_FILENO);\r
 163 > +\r
 164 > +             /* This process is now out of reach of shell's job control.\r
 165 > +                To resolve the rare but possible condition where this\r
 166 > +                "daemon" is started but never connected this process will\r
 167 > +                (only) have 30 seconds to exist. */\r
 168 > +             alarm (30);\r
 169 \r
 170 30 seconds is probably enough. If it turns out that we need longer\r
 171 timeout in some scenario, I'd suggest to call alarm() also when\r
 172 something is received on the socket to extend the timeout.\r
 173 \r
 174 Overall, both patches LGTM.\r
 175 \r
 176 -Michal\r