Re: [PATCH v3] send-email: --batch-size to work around some SMTP server limit

From: Junio C Hamano <gitster@pobox.com>
Date: May 8, 2017, 04:11 UTC
Message-ID: <xmqqpofk9edz.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20170507043648.24143-1-zxq_yx_007@163.com>

xiaoqiang zhao <zxq_yx_007@163.com> writes:

Show 10 quoted lines

> @@ -1664,6 +1674,14 @@ foreach my $t (@files) {
>  		}
>  	}
>  	$message_id = undef;
> +	$num_sent++;
> +	if ($num_sent == $batch_size) {
> +		$num_sent = 0;
> +		$smtp->quit;
> +		$smtp = undef;
> +		$auth = 0;

Two suggestions.

 (1) I do not think $smtp is always valid when we come here; it is
     unsafe to unconditionally say $smtp->quit like this patch does.

	$smtp->quit if defined $smtp;

     may help codepaths like $dry_run and also the case where
     $smtp_server is the absolute path to a local program.

 (2) You are setting $auth to zero to force re-authentication to
     happen.  It would be more consistent to the state of $auth that
     is not-yet-used to "undef $auth;" here instead.  After all, the
     variable starts its life in an undefined state.

So all in all

	$smtp->quit if defined $smtp;
	undef $smtp;
	undef $auth;

perhaps?

This change of course forces re-authentication every N messages, which may not hurt those who use some form of credential helper, but that may be something we want to mention in the log message.

> +		sleep($relogin_delay);
> +	}
>  }

Thanks.