From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 30888 invoked by alias); 11 Dec 2014 17:04:35 -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 30859 invoked by uid 89); 11 Dec 2014 17:04:32 -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,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: e06smtp11.uk.ibm.com Received: from e06smtp11.uk.ibm.com (HELO e06smtp11.uk.ibm.com) (195.75.94.107) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-SHA encrypted) ESMTPS; Thu, 11 Dec 2014 17:04:31 +0000 Received: from /spool/local by e06smtp11.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Thu, 11 Dec 2014 17:04:28 -0000 Received: from d06dlp01.portsmouth.uk.ibm.com (9.149.20.13) by e06smtp11.uk.ibm.com (192.168.101.141) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; Thu, 11 Dec 2014 17:04:27 -0000 Received: from b06cxnps3075.portsmouth.uk.ibm.com (d06relay10.portsmouth.uk.ibm.com [9.149.109.195]) by d06dlp01.portsmouth.uk.ibm.com (Postfix) with ESMTP id 1446B17D8043 for ; Thu, 11 Dec 2014 17:04:50 +0000 (GMT) Received: from d06av10.portsmouth.uk.ibm.com (d06av10.portsmouth.uk.ibm.com [9.149.37.251]) by b06cxnps3075.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id sBBH4R2B58916968 for ; Thu, 11 Dec 2014 17:04:27 GMT Received: from d06av10.portsmouth.uk.ibm.com (localhost [127.0.0.1]) by d06av10.portsmouth.uk.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id sBBH4Rid026514 for ; Thu, 11 Dec 2014 10:04:27 -0700 Received: from br87z6lw.de.ibm.com (dyn-9-152-212-196.boeblingen.de.ibm.com [9.152.212.196]) by d06av10.portsmouth.uk.ibm.com (8.14.4/8.14.4/NCO v10.0 AVin) with ESMTP id sBBH4QHZ026496; Thu, 11 Dec 2014 10:04:26 -0700 From: Andreas Arnez To: "Ulrich Weigand" Cc: gdb-patches@sourceware.org Subject: Re: [PATCH v3 3/3] S390: Fix gdbserver support for TDB References: <201412101916.sBAJG9x7025122@d03av02.boulder.ibm.com> Date: Thu, 11 Dec 2014 17:04:00 -0000 In-Reply-To: <201412101916.sBAJG9x7025122@d03av02.boulder.ibm.com> (Ulrich Weigand's message of "Wed, 10 Dec 2014 20:16:08 +0100 (CET)") Message-ID: <87d27qm82t.fsf@br87z6lw.de.ibm.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-TM-AS-MML: disable X-Content-Scanned: Fidelis XPS MAILER x-cbid: 14121117-0005-0000-0000-0000026DA25E X-IsSubscribed: yes X-SW-Source: 2014-12/txt/msg00247.txt.bz2 On Wed, Dec 10 2014, Ulrich Weigand wrote: > Andreas Arnez wrote: > >> @@ -4256,7 +4256,7 @@ regsets_fetch_inferior_registers (struct regsets_info *regsets_info, >> this process mode. */ >> disable_regset (regsets_info, regset); >> } >> - else >> + else if (errno != ENODATA) >> { >> char s[256]; >> sprintf (s, "ptrace(regsets_fetch_inferior_registers) PID=%d", > > It would be better to keep the comment explaining in what situations the kernel > can return ENODATA that you had in a previous iteration of the patch set. OK, I will add this comment in the next version: else if (errno == ENODATA) { /* ENODATA may be returned if the regset is currently not "active". This can happen in normal operation, so suppress the warning in this case. */ } else ... > >> @@ -4293,7 +4293,8 @@ regsets_store_inferior_registers (struct regsets_info *regsets_info, >> void *buf, *data; >> int nt_type, res; >> >> - if (regset->size == 0 || regset_disabled (regsets_info, regset)) >> + if (regset->size == 0 || regset_disabled (regsets_info, regset) >> + || regset->fill_function == NULL) >> continue; > > This (and the related s390_fill_last_break change) is really an independent > change; maybe do it as a separate patch? For consistency, we might likewise > want to allow regsets with NULL store_function (in regsets_fetch_inferior_registers). OK, I will split the change to allow regsets with a NULL store_function and the exploitation for last_break out into a separate patch ("gdbserver: Support read-only regsets"). And for consistency I can also allow regsets with a NULL store_function. In that case I *think* we should suppress the invocation of ptrace with the regset's get_request in regsets_store_inferior_registers(). In other words, instead of read-modify-write we would then only do the write. Agreed? In the original patch I omitted support for write-only regsets because I do not see a use case for this; and if there will be a use case in the future, I am not sure that the approach described above will really be appropriate.