Return-Path: <sojkam1@fel.cvut.cz>
X-Original-To: notmuch@notmuchmail.org
Delivered-To: notmuch@notmuchmail.org
Received: from localhost (localhost [127.0.0.1])
	by olra.theworths.org (Postfix) with ESMTP id 385B9431FAF
	for <notmuch@notmuchmail.org>; Thu, 20 Sep 2012 09:38:04 -0700 (PDT)
X-Virus-Scanned: Debian amavisd-new at olra.theworths.org
X-Spam-Flag: NO
X-Spam-Score: -2.3
X-Spam-Level: 
X-Spam-Status: No, score=-2.3 tagged_above=-999 required=5
	tests=[RCVD_IN_DNSWL_MED=-2.3] autolearn=disabled
Received: from olra.theworths.org ([127.0.0.1])
	by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024)
	with ESMTP id OADjm0cGuJWs for <notmuch@notmuchmail.org>;
	Thu, 20 Sep 2012 09:38:03 -0700 (PDT)
Received: from max.feld.cvut.cz (max.feld.cvut.cz [147.32.192.36])
	by olra.theworths.org (Postfix) with ESMTP id C1FEA431FAE
	for <notmuch@notmuchmail.org>; Thu, 20 Sep 2012 09:38:02 -0700 (PDT)
Received: from localhost (unknown [192.168.200.4])
	by max.feld.cvut.cz (Postfix) with ESMTP id CBE9F19F33B0;
	Thu, 20 Sep 2012 18:37:57 +0200 (CEST)
X-Virus-Scanned: IMAP AMAVIS
Received: from max.feld.cvut.cz ([192.168.200.1])
	by localhost (styx.feld.cvut.cz [192.168.200.4]) (amavisd-new,
	port 10044)
	with ESMTP id X4nqNXQtRn0q; Thu, 20 Sep 2012 18:37:55 +0200 (CEST)
Received: from imap.feld.cvut.cz (imap.feld.cvut.cz [147.32.192.34])
	by max.feld.cvut.cz (Postfix) with ESMTP id DE76E19F3392;
	Thu, 20 Sep 2012 18:37:55 +0200 (CEST)
Received: from steelpick.2x.cz (note-sojka.felk.cvut.cz [147.32.86.30])
	(Authenticated sender: sojkam1)
	by imap.feld.cvut.cz (Postfix) with ESMTPSA id BC95E660904;
	Thu, 20 Sep 2012 18:37:52 +0200 (CEST)
Received: from wsh by steelpick.2x.cz with local (Exim 4.80)
	(envelope-from <sojkam1@fel.cvut.cz>)
	id 1TEjl9-0003E9-Ub; Thu, 20 Sep 2012 18:37:51 +0200
From: Michal Sojka <sojkam1@fel.cvut.cz>
To: Tomi Ollila <tomi.ollila@iki.fi>, notmuch@notmuchmail.org
Subject: Re: [PATCH V3 1/2] test/smtp-dummy: add --background option and
	functionality
In-Reply-To: <1347978182-8771-1-git-send-email-tomi.ollila@iki.fi>
References: <1347978182-8771-1-git-send-email-tomi.ollila@iki.fi>
User-Agent: Notmuch/0.14+23~g9d68aca (http://notmuchmail.org) Emacs/24.2.1
	(x86_64-pc-linux-gnu)
Date: Thu, 20 Sep 2012 18:37:51 +0200
Message-ID: <87zk4k4pls.fsf@steelpick.2x.cz>
MIME-Version: 1.0
Content-Type: text/plain
Cc: Tomi Ollila <too@iki.fi>
X-BeenThere: notmuch@notmuchmail.org
X-Mailman-Version: 2.1.13
Precedence: list
List-Id: "Use and development of the notmuch mail system."
	<notmuch.notmuchmail.org>
List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,
	<mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>
List-Archive: <http://notmuchmail.org/pipermail/notmuch>
List-Post: <mailto:notmuch@notmuchmail.org>
List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>
List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,
	<mailto:notmuch-request@notmuchmail.org?subject=subscribe>
X-List-Received-Date: Thu, 20 Sep 2012 16:38:04 -0000

Hi Tomi,

On Tue, Sep 18 2012, Tomi Ollila wrote:
> From: Tomi Ollila <too@iki.fi>
>
> When shell executes background process using '&' the scheduling of
> that new process is arbitrary. It could be that smtp-dummy doesn't
> get execution time to listen() it's server socket until some other
> process attempts to connect() to it. The --background option in
> smtp-dummy makes it to go background *after* it started to listen
> its server socket.
>
> When --background option is used, the line "smtp_dummy_pid='<pid>'"
> is printed to stdout from where shell can eval it.
> ---
>
> This is v3 of id:"1323766883-17607-1-git-send-email-tomi.ollila@iki.fi"
>
> addressing (some) Dmitry's comments.
>
>  test/smtp-dummy.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 54 insertions(+), 1 deletion(-)
>
> diff --git a/test/smtp-dummy.c b/test/smtp-dummy.c
> index 86d4316..bb13668 100644
> --- a/test/smtp-dummy.c
> +++ b/test/smtp-dummy.c
> @@ -119,6 +119,7 @@ do_smtp_to_file (FILE *peer, FILE *output)
>  int
>  main (int argc, char *argv[])
>  {
> +	const char * progname;
>  	char *output_filename;
>  	FILE *peer_file, *output;
>  	int sock, peer, err;
> @@ -126,9 +127,31 @@ main (int argc, char *argv[])
>  	struct hostent *hostinfo;
>  	socklen_t peer_addr_len;
>  	int reuse;
> +	int background;
> +
> +	progname = argv[0];
> +
> +	background = 0;
> +	for (; argc >= 2; argc--, argv++) {
> +		if (argv[1][0] != '-')

This is ugly, but all right. I do not think we need to have high
standards for this piece of code :)

> +			break;
> +		if (strcmp (argv[1], "--") == 0) {
> +			argc--;
> +			argv++;
> +			break;
> +		}
> +		if (strcmp (argv[1], "--background") == 0) {
> +			background = 1;
> +			continue;
> +		}
> +		fprintf(stderr, "%s: unregognized option '%s'\n",
> +			progname, argv[1]);
> +		return 1;
> +	}
>
>  	if (argc != 2) {
> -		fprintf (stderr, "Usage: %s <output-file>\n", argv[0]);
> +		fprintf (stderr,
> +			 "Usage: %s [--background] <output-file>\n", progname);
>  		return 1;
>  	}
>
> @@ -181,6 +204,36 @@ main (int argc, char *argv[])
>  		return 1;
>  	}
>
> +	if (background) {
> +		int pid = fork ();
> +		if (pid > 0) {
> +			printf ("smtp_dummy_pid='%d'\n", pid);
> +			fflush (stdout);
> +			close (sock);
> +			return 0;
> +		}
> +		if (pid < 0) {
> +			fprintf (stderr, "Error: fork() failed: %s\n",
> +				 strerror (errno));
> +			close (sock);
> +			return 1;
> +		}
> +		/* Reached if pid == 0 (the child process). */
> +		/* Close stdout so that the one interested in pid value will
> +		   also get EOF. */
> +		close (STDOUT_FILENO);
> +		/* dup2() will re-reserve fd of stdout (1) (opportunistically),
> +		   in case fd of stderr (2) is open. If that was not open we
> +		   don't care fd of stdout (1) either. */
> +		dup2 (STDERR_FILENO, STDOUT_FILENO);
> +
> +		/* This process is now out of reach of shell's job control.
> +		   To resolve the rare but possible condition where this
> +		   "daemon" is started but never connected this process will
> +		   (only) have 30 seconds to exist. */
> +		alarm (30);

30 seconds is probably enough. If it turns out that we need longer
timeout in some scenario, I'd suggest to call alarm() also when
something is received on the socket to extend the timeout.

Overall, both patches LGTM.

-Michal