From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [63.128.21.124]) by sourceware.org (Postfix) with ESMTP id 4517E38708C5 for ; Thu, 11 Mar 2021 19:23:28 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 4517E38708C5 Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-262-6h4mbiphNZyxbXAVpfwqng-1; Thu, 11 Mar 2021 14:23:26 -0500 X-MC-Unique: 6h4mbiphNZyxbXAVpfwqng-1 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 149B4100D671; Thu, 11 Mar 2021 19:23:25 +0000 (UTC) Received: from oldenburg.str.redhat.com (ovpn-112-77.ams2.redhat.com [10.36.112.77]) by smtp.corp.redhat.com (Postfix) with ESMTPS id CC58650A8A; Thu, 11 Mar 2021 19:23:23 +0000 (UTC) From: Florian Weimer To: Adhemerval Zanella Cc: Andreas Schwab , Adhemerval Zanella via Libc-alpha Subject: Re: [PATCH 5/8] posix: Do not clobber errno by atfork handlers References: <20210202151134.2123748-1-adhemerval.zanella@linaro.org> <20210202151134.2123748-5-adhemerval.zanella@linaro.org> <87o8fs4mm9.fsf@oldenburg.str.redhat.com> <09b94a9b-905d-db39-3566-767c097f4348@linaro.org> <87eeglizzs.fsf@oldenburg.str.redhat.com> <13c1767e-a4b5-788f-611e-3c660fbceef6@linaro.org> <87czw5ojm9.fsf@igel.home> <87mtv9hine.fsf@oldenburg.str.redhat.com> <405c0b7b-b7ae-edde-8354-b69442ba6348@linaro.org> <87im5xemfa.fsf@oldenburg.str.redhat.com> Date: Thu, 11 Mar 2021 20:23:34 +0100 In-Reply-To: (Adhemerval Zanella's message of "Thu, 11 Mar 2021 14:25:32 -0300") Message-ID: <87blbpcx55.fsf@oldenburg.str.redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain X-Spam-Status: No, score=-6.5 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 11 Mar 2021 19:23:29 -0000 * Adhemerval Zanella: > On 11/03/2021 12:32, Florian Weimer wrote: >> * Adhemerval Zanella: >> >>>>>> if (pid < 0) >>>>>> __set_errno (save_errno) >>>> >>>> Agreed. And then it's clear it should be moved to the parent branch of >>>> the if statement above. >>>> >>> >>> But moving it before __run_fork_handlers in the the previous if (which >>> handles the pid != 0) will both 1. reset the errno even for success >>> (pid > 0) and 2. not fixing the issue since __run_fork_handlers will >>> be issue later. What I am missing in your suggestion? >> >> After the __run_fork_handlers call on the parent branch. > > Why is this an improvement over doing on fork.c, now that with > this change is the only caller of __run_fork_handlers and the > routine with the information (pid result) to restore the > errno? I suggest to leave it in fork.c, but put it in the parent branch, instead after the if statement. The code only runs if fork fails, so it's something the parent process needs to do. It's not cleanup that has to happen on both paths. The compiler will likely optimize it to identical machine code, but I think it's helpful to the reader to put it on the parent execution path. Thanks, Florian