From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on inbox.vuxu.org X-Spam-Level: X-Spam-Status: No, score=-1.0 required=5.0 tests=MAILING_LIST_MULTI, RCVD_IN_DNSWL_NONE autolearn=ham autolearn_force=no version=3.4.2 Received: from primenet.com.au (ns1.primenet.com.au [203.24.36.2]) by inbox.vuxu.org (OpenSMTPD) with ESMTP id 094fa4ee for ; Mon, 17 Feb 2020 14:12:23 +0000 (UTC) Received: (qmail 21313 invoked by alias); 17 Feb 2020 14:12:18 -0000 Mailing-List: contact zsh-workers-help@zsh.org; run by ezmlm Precedence: bulk X-No-Archive: yes List-Id: Zsh Workers List List-Post: List-Help: List-Unsubscribe: X-Seq: 45449 Received: (qmail 27657 invoked by uid 1010); 17 Feb 2020 14:12:18 -0000 X-Qmail-Scanner-Diagnostics: from mail-qk1-f193.google.com by f.primenet.com.au (envelope-from , uid 7791) with qmail-scanner-2.11 (clamdscan: 0.102.2/25725. spamassassin: 3.4.2. Clear:RC:0(209.85.222.193):SA:0(-2.0/5.0):. Processed in 0.771043 secs); 17 Feb 2020 14:12:18 -0000 X-Envelope-From: chris@chrisdown.name X-Qmail-Scanner-Mime-Attachments: | X-Qmail-Scanner-Zip-Files: | Received-SPF: pass (ns1.primenet.com.au: SPF record at _netblocks.google.com designates 209.85.222.193 as permitted sender) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:content-transfer-encoding :in-reply-to; bh=RX/Ra7RYmOw3vOmFQXBSmDZpcELcAuNutzGy95o0JbI=; b=j81G+eW8F8klBo7IzGVNX4fUo/02lxRpPZ2+3m3i+oakkqETWf0ucpvrO5cB+81qdD NDMcZ2gd0HkD20uCqXf0yqVNrEQv5IIXUpFnec0kFKn36fGhNr81xAEobgKXNFYbCVgW vqeo+Twpu6T2jVaQ0uk7QH4SVoyywFwh/2IW6PizKd4XLbB+XHSEQu2sLUNRbsofvI9u owDdQqazjjN3fBLV4a/hZXFnIZL9fN0CLqlnjty/IsYjva8z+1J9CFXf3CUSe3VDt6S0 YEP2yWXP5xBDdrdytfwQ7vUPo+zQesbKjAIXRJKHZlTAeCmQPYCm1/X/pC0EX1zPeRuv UsAw== X-Gm-Message-State: APjAAAUn9suH7Hiz+RgPzx7rea4uFLal3OyMu/JJmc6B+byjKMk4R+WX qhgzZ00zsaEwPSJnYnMZPAKn9+r9OCVOFQ== X-Google-Smtp-Source: APXvYqyX48NFHirWZj3vWyv+bja4tWr1riqU6vz1A2bA8gfo/E4NoCj8FAZ0nrNxwGuIHDpbHQuhTA== X-Received: by 2002:ae9:c317:: with SMTP id n23mr14508666qkg.356.1581948704888; Mon, 17 Feb 2020 06:11:44 -0800 (PST) Date: Mon, 17 Feb 2020 09:11:44 -0500 From: Chris Down To: Daniel Shahaf Cc: zsh-workers@zsh.org Subject: Re: [PATCH v2 2/2] builtins: kill: Do not signal current process group when pid is empty Message-ID: <20200217141144.GB20166@chrisdown.name> References: <20200216103954.GC15773@tarpaulin.shahaf.local2> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1; format=flowed Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20200216103954.GC15773@tarpaulin.shahaf.local2> Daniel Shahaf writes: >Chris Down wrote on Sat, Feb 15, 2020 at 20:21:09 -0400: >> 1. Add special handling to `kill` to avoid this case. See this patch[0] >> for a version that does that. > >> 0: https://www.zsh.org/mla/workers/2020/msg00251.html > >Suggest to state "workers/45426" next to the link: that's how we customarily >refer to mailing lists posts. The number comes from the X-Seq header and is >shown in the archives too. Sure thing. :-) >> +++ b/Test/B11kill.ztst >> @@ -50,3 +50,15 @@ >> kill -INT >> 1:kill with sigspec only >> ?(eval):kill:1: not enough arguments >> + >> +# Regression tests: `kill ''` should not result in `kill 0`. >> + >> + trap 'exit 19' TERM >> + kill '' >> +1:Plain kill with empty pid should not send signal to current process group >> +?(eval):kill:2: illegal pid: > >Should this test use a subshell so as not to kill the test suite process in >case the bug regresses? > >Personally, I'm partial to first adding commits in "expected to fail" mode >(using the «f» flag after the expected exit code) and then in a separate commit >fix the bug, since doing so in effect validates the test. However, I see this >as a nice-to-hvae, not a blocker. For this particular one, the subshell won't help, since we're part of the same process group as make and the test runner regardless: make: *** [Makefile:263: check] User defined signal 1 make[1]: *** [Makefile:190: check] User defined signal 1 make[1]: Leaving directory '/home/cdown/git/zsh/Test' One possibility to avoid that is to run these few tests in a shell that's a new session leader, like in W02jobs, I suppose. What do you think about that? > >> + trap 'exit 11' INT >> + kill -INT '' >> +1:kill with empty pid and sigspec should not send signal to current process group >> +?(eval):kill:2: illegal pid: > >Could you add a comment explaining the magic numbers 11 and 19? Sure thing, will do in v3. Thanks! Chris