From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 55958 invoked by alias); 17 Apr 2015 16:47:19 -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 55946 invoked by uid 89); 17 Apr 2015 16:47:19 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.6 required=5.0 tests=AWL,BAYES_00,KAM_LAZY_DOMAIN_SECURITY,SPF_HELO_PASS,T_RP_MATCHES_RCVD autolearn=no 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; Fri, 17 Apr 2015 16:47:18 +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 (Postfix) with ESMTPS id 13945AD02B for ; Fri, 17 Apr 2015 16:47:17 +0000 (UTC) Received: from blade.nx (ovpn-116-95.ams2.redhat.com [10.36.116.95]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t3HG5Ple010023; Fri, 17 Apr 2015 12:05:25 -0400 Received: by blade.nx (Postfix, from userid 1000) id 7C10226410C; Fri, 17 Apr 2015 17:05:24 +0100 (BST) Date: Fri, 17 Apr 2015 16:47:00 -0000 From: Gary Benson To: Pedro Alves Cc: gdb-patches@sourceware.org Subject: Re: [PATCH 7/7] Implement vFile:setfs in gdbserver Message-ID: <20150417160524.GC14618@blade.nx> References: <1429186791-6867-1-git-send-email-gbenson@redhat.com> <1429186791-6867-8-git-send-email-gbenson@redhat.com> <553126FA.8030402@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <553126FA.8030402@redhat.com> X-IsSubscribed: yes X-SW-Source: 2015-04/txt/msg00696.txt.bz2 Pedro Alves wrote: > On 04/16/2015 01:19 PM, Gary Benson wrote: > > +/* Handle a "vFile:setfs:" packet. */ > > + > > +static void > > +handle_setfs (char *own_buf) > > +{ [snip] > > +} > > This should probably return empty if the_target->call_with_fs_of is > NULL, to disable the packet in GDB? > > > int > > handle_vFile (char *own_buf, int packet_len, int *new_packet_len) > > { > > - if (startswith (own_buf, "vFile:open:")) > > + if (the_target->call_with_fs_of != NULL > > + && startswith (own_buf, "vFile:setfs:")) > > + handle_setfs (own_buf); > > + else if (startswith (own_buf, "vFile:open:")) > > Ah, I see now that it's handled here. I'd prefer moving that > check inside the handle_setfs function instead so that the "setfs" > specifics handling is all localized. Ok. > > +struct open_closure > > +{ > > + char filename[HOSTIO_PATH_MAX]; > > + int flags; > > + int mode; > > mode_t mode. > > (please check for the the same issue on the native side.) It's there in both places. Is mode_t ok to use in gdbserver? I don't see it anywhere... > > + int return_value; > > +}; > > > +/* Implementation of target_ops->call_with_fs_of. */ > > + > > +static int > > +linux_low_call_with_fs_of (int pid, void (*func) (void *), void *arg) > > +{ > > + return linux_ns_enter (pid, LINUX_NS_MNT, func, arg); > > +} > > + > > So back on linux_ns_enter's naming --- > > I find the abstraction here a bit odd. It seems to be me that > a function that does: > > #1 - select filesystem/namespace > #2 - call foo > #3 - restore filesystem/namespace > > should be a function in common code, and that the only > target-specific part is selecting a filesystem/namespace. > That is, simplifying, something like: > > /* Cause the filesystem to appear as it does to process PID and > call FUNC with argument ARG, restoring the filesystem to its > original state afterwards. Return nonzero if FUNC was called, > zero otherwise (and set ERRNO). */ > > int > call_with_fs_of (int pid, void (*func) (void *), void *arg) > { > int ret; > > target->select_fs (pid); > ret = func (); > target->select_fs (0); > return ret; > } > > Was there a reason this wasn't done that way? > > (maybe make target->select_fs() return the previous > namespace, instead of passing 0 on the second > call.) I'll have a go at doing it that way. Thanks, Gary -- http://gbenson.net/