From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 108283 invoked by alias); 31 Jul 2017 19:08:36 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 108246 invoked by uid 89); 31 Jul 2017 19:08:35 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.3 required=5.0 tests=AWL,BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.2 spammy=hoc, clever, Hx-languages-length:1740 X-HELO: smtp.polymtl.ca Received: from smtp.polymtl.ca (HELO smtp.polymtl.ca) (132.207.4.11) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 31 Jul 2017 19:08:34 +0000 Received: from simark.ca (simark.ca [158.69.221.121]) (authenticated bits=0) by smtp.polymtl.ca (8.14.7/8.14.7) with ESMTP id v6VJ8RTX013886 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Mon, 31 Jul 2017 15:08:32 -0400 Received: by simark.ca (Postfix, from userid 112) id 9E3DF1EA08; Mon, 31 Jul 2017 15:08:27 -0400 (EDT) Received: from simark.ca (localhost [127.0.0.1]) by simark.ca (Postfix) with ESMTP id 0CECD1E5B1; Mon, 31 Jul 2017 15:08:26 -0400 (EDT) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII; format=flowed Content-Transfer-Encoding: 7bit Date: Mon, 31 Jul 2017 19:08:00 -0000 From: Simon Marchi To: Tom Tromey Cc: Pedro Alves , gdb-patches@sourceware.org Subject: Re: [RFA 09/23] Remove close cleanup In-Reply-To: <87iniom59v.fsf@tromey.com> References: <20170503224626.2818-1-tom@tromey.com> <20170503224626.2818-10-tom@tromey.com> <87iniom59v.fsf@tromey.com> Message-ID: X-Sender: simon.marchi@polymtl.ca User-Agent: Roundcube Webmail/1.3.0 X-Poly-FromMTA: (simark.ca [158.69.221.121]) at Mon, 31 Jul 2017 19:08:27 +0000 X-IsSubscribed: yes X-SW-Source: 2017-07/txt/msg00451.txt.bz2 On 2017-07-20 00:27, Tom Tromey wrote: >>>>>> "Pedro" == Pedro Alves writes: > > Pedro> Patch looks good. > Pedro> The bit below made me stop for a second: > [...] > > > Pedro> Would the following be too clever? > Pedro> /* Restore errno on exit to the value saved by the > Pedro> last save() call. Ctor saves. */ > Pedro> struct scoped_errno_restore > [...] > > Pedro> /* Save errno and return MNSH_FS_ERROR. */ > Pedro> auto fs_err = [&] () > Pedro> { > Pedro> restore_errno.save (); > Pedro> return MNSH_FS_ERROR; > Pedro> }; > [...] > > Pedro> [It gets rid of both the scope, and the gotos.] > > How about making fd_closer a template, like: > > template > class fd_closer > ... > ~fd_closer () > { > if (m_fd != -1) > CLOSER (m_fd); > } > > Then in linux-namespaces.c: > > /* A function like "close" that saves and restores errno. */ > > static int > close_saving_fd (int fd) > { > scoped_restore save_errno = make_scoped_restore (&errno); > return close (fd); > } > > ... > > gdb::fd_closer close_fd (fd); > > > This also removes the scope and the gotos. > > The main plus is that it's simpler. The main minus is that it's ad > hoc. > > Tom (Responding here although I am reviewing v2) Instead of finding a solution to preserve errno, I think we should return it explicitly when needed, it would be more robust and easier to follow the trail where the value comes from. That would mean changing static enum mnsh_fs_code linux_mntns_access_fs (pid_t pid) to static enum mnsh_fs_code linux_mntns_access_fs (pid_t pid, int *errnop) and possibly others. In that case, *ERRNOP would be set if MNSH_FS_ERROR is returned. Simon