From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 105455 invoked by alias); 21 Apr 2017 20:43:08 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Received: (qmail 105397 invoked by uid 89); 21 Apr 2017 20:43:06 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-24.6 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_NONE,RCVD_IN_SORBS_SPAM,SPF_PASS autolearn=ham version=3.3.2 spammy=recall X-HELO: mail-qk0-f171.google.com X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:cc:from:message-id:date :user-agent:mime-version:in-reply-to:content-transfer-encoding; bh=co4GhpHxzbuIkl8eWwe7ZY6NiBTKbKQliw+NGitVHHc=; b=Ya2GAvcGgXON724KVvitByHTUHkdCPwhL/ov4VKSsdbKXteiado0nAhh7PI6cnK35h DYqVUtLkOg98wKqrv1mXOGwbBpg17U06bPgE+2RLZjKwjnOmxOdGnoG92x6QHqfjB25w CWIxIrd3UPI0vropkeUNzjgyITAkKBWF5+vpbODxh8PNtWfaylGh3CkgiW6vlYgSp4Rl Lv/UNt/rfH/4/S9WvEqseJwxa93Oc3YP6rRu04SOcBqprlC7hNfdg1Cl+4eGqgNQqF9D d9Qo1a0hVtY9OQqG6QwinJObpRf+uaJBO6e8UMeZ8Gke2bt/xWr+Bmv7fj4y8f/ov3xs ngjw== X-Gm-Message-State: AN3rC/6DjoxVHQIyBEPfWKDmu/jIB38C7UjQNEwSb2bvY9mEBApx5gXr +ElnuzHmq8wkDVHT X-Received: by 10.55.217.155 with SMTP id q27mr3860711qkl.217.1492807384325; Fri, 21 Apr 2017 13:43:04 -0700 (PDT) Subject: Re: [PATCH v3][BZ 21340] add support for POSIX_SPAWN_SETSID To: Florian Weimer References: <20170405054116.9007-1-quae@daurnimator.com> <75eb12d0-a3f1-2b37-90d5-0a9a3f7f9a99@redhat.com> <8760hxv865.fsf@mid.deneb.enyo.de> Cc: Florian Weimer , libc-alpha@sourceware.org, Zack Weinberg From: Adhemerval Zanella Message-ID: <511674c7-5be6-1438-ccab-962cfc179b45@linaro.org> Date: Fri, 21 Apr 2017 20:43:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: <8760hxv865.fsf@mid.deneb.enyo.de> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-SW-Source: 2017-04/txt/msg00485.txt.bz2 On 21/04/2017 17:04, Florian Weimer wrote: > * Adhemerval Zanella: > >> +static int >> +do_test_setsid (bool test_setsid) > > This should return void. Ack. > >> + if (test_setsid) >> + { >> + if (posix_spawnattr_setflags (&attrp, POSIX_SPAWN_SETSID)) >> + FAIL_EXIT1 ("posix_spawnattr_setflags: %m"); >> + } > > You need to set errno before you can use %m. Ack. > >> + res = posix_spawnp (&child, "true", NULL, &attrp, args, environ); >> + /* posix_spawnattr_destroy is noop. */ >> + posix_spawnattr_destroy (&attrp); >> + >> + if (res != 0) >> + FAIL_EXIT1 ("posix_spawnp: %m"); > > Likewise. Ack. > >> +static int >> +do_test (void) >> +{ >> + int ret = 0; >> + >> + ret += do_test_setsid (false); >> + ret += do_test_setsid (true); >> + >> + return 0; >> +} > > ret is effectively unused. Yeah, I though about doing that since we bail out with an exit for failure. I will change it. > >> diff --git a/sysdeps/unix/sysv/linux/spawni.c b/sysdeps/unix/sysv/linux/spawni.c >> index d7f9e83..3cf77d5 100644 >> --- a/sysdeps/unix/sysv/linux/spawni.c >> +++ b/sysdeps/unix/sysv/linux/spawni.c > >> + if ((attr->__flags & POSIX_SPAWN_SETSID) != 0 >> + && (ret = __setsid ()) < 0) >> + goto fail; > > I believe the assignment to ret is dead. > >> + >> /* Set the process group ID. */ >> if ((attr->__flags & POSIX_SPAWN_SETPGROUP) != 0 >> && (ret = __setpgid (0, attr->__pgrp)) != 0) > > Apparently, this is an existing problem with the code ... > If I recall correctly, on initial iterations for posix_spawn rewrite I used the ret return to signal the error, but it seems later I generalize to use errno instead. I will fix this usage, but I think extra fixes should go to on a different patch.