From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 93618 invoked by alias); 18 Mar 2015 18:45:48 -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 93585 invoked by uid 89); 18 Mar 2015 18:45:47 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.1 required=5.0 tests=AWL,BAYES_00,SPF_HELO_PASS,SPF_PASS,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Wed, 18 Mar 2015 18:45:41 +0000 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id t2IIjeaJ021294 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL) for ; Wed, 18 Mar 2015 14:45:40 -0400 Received: from blade.nx (ovpn-116-79.ams2.redhat.com [10.36.116.79]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t2IIjdMZ019992; Wed, 18 Mar 2015 14:45:40 -0400 Received: by blade.nx (Postfix, from userid 1000) id 0203D265074; Wed, 18 Mar 2015 18:45:38 +0000 (GMT) Date: Wed, 18 Mar 2015 18:45:00 -0000 From: Gary Benson To: Pedro Alves Cc: gdb-patches@sourceware.org Subject: Re: [PATCH] Encapsulate target_fileio file descriptors Message-ID: <20150318184538.GA32134@blade.nx> References: <1426684854-20330-1-git-send-email-gbenson@redhat.com> <55099F05.1010501@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <55099F05.1010501@redhat.com> X-IsSubscribed: yes X-SW-Source: 2015-03/txt/msg00552.txt.bz2 Pedro Alves wrote: > On 03/18/2015 01:20 PM, Gary Benson wrote: > > Various target_fileio_* functions use integer file descriptors to > > refer to open files, which can cause problems if the target stack > > changes between the file being opened and other file operations > > being performed on that file. This commit makes the > > target_fileio_* functions that use file descriptors use an opaque > > target_fileio_t handle instead that encapsulates both the file > > descriptor itself and the target_ops vector that should be used > > to access it. > > Thanks Gary. > > I think this deserves an expanded explanation, for posterity. ... > And obviously, if FD happens to be a file descriptor number that > happens to be open in GDB as well, we'll close it by mistake. > That's the sort of random issue that wouldn't be very fun to track > down! Yeah, like GDB printing 350MB of poll errors because it closed one of its readline pipes :) > It works today, though I suspect that it may cause problems in > the future. When we get to multi-target, and target_ops instances > become objects instantiated on the heap, we'll have the problem that > the target_ops pointer in the handle becomes invalid as soon as the > target_ops object is closed/destroyed/released (unless the target > object is refcounted, which isn't clear it should). > > A table-based solution, where target.c maintains (something > conceptually like) a table of "target file descriptor + target_ops" > elements, and passes an index into that table as file handle to > target_fileio_ callers, would make it possible to invalidate the > file handles directly in the table when a target is closed, so > that e.g., target_fileio_close (etc.) could detect that the file > handle is now invalid, and return error with errno=EBADF _without_ > calling through the now invalid target_ops pointer. The files could be closed when the table entry is invalidated too. I'll reimplement this and post a new patch tomorrow or Friday. > Speaking of EBADF, I noticed that remote_hostio_send_command > fails with ENOSYS if the remote connection is closed, which > seems wrong to me. That seems intuitive for the ones that take file descriptor arguments (remote_hostio_{pread,write,fstat,close}). Not sure if that makes sense for the ones that take filenames (open, readlink, unlink). ENOSYS isn't really correct for those either--they are implemented, you just can't do it right now. ECOMM? ECONNABORTED? ENOTCONN? EPIPE? EREMOTEIO? I'm open to suggestions :) Cheers, Gary -- http://gbenson.net/