From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 117279 invoked by alias); 10 Apr 2019 11:42: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 117270 invoked by uid 89); 10 Apr 2019 11:42:35 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.1 spammy=showing X-HELO: mail-wr1-f67.google.com Received: from mail-wr1-f67.google.com (HELO mail-wr1-f67.google.com) (209.85.221.67) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 10 Apr 2019 11:42:34 +0000 Received: by mail-wr1-f67.google.com with SMTP id y7so2513716wrn.11 for ; Wed, 10 Apr 2019 04:42:33 -0700 (PDT) Return-Path: Received: from ?IPv6:2001:8a0:f913:f700:56ee:75ff:fe8d:232b? ([2001:8a0:f913:f700:56ee:75ff:fe8d:232b]) by smtp.gmail.com with ESMTPSA id o5sm2427962wmc.16.2019.04.10.04.42.30 (version=TLS1_3 cipher=AEAD-AES128-GCM-SHA256 bits=128/128); Wed, 10 Apr 2019 04:42:31 -0700 (PDT) Subject: Re: [PATCH] Fix amd64->i386 linux syscall restart problem To: Kevin Buettner , gdb-patches@sourceware.org References: <20190316221341.021f7c62@f29-4.lan> <975e7120-1b61-2807-5aab-b961e07a80d0@redhat.com> <20190409192916.79fcb539@f29-4.lan> From: Pedro Alves Message-ID: Date: Wed, 10 Apr 2019 11:42:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.2.1 MIME-Version: 1.0 In-Reply-To: <20190409192916.79fcb539@f29-4.lan> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-SW-Source: 2019-04/txt/msg00178.txt.bz2 On 4/10/19 3:29 AM, Kevin Buettner wrote: > On Tue, 9 Apr 2019 18:46:45 +0100 > Pedro Alves wrote: > >>> + void *ptr = (gdb_byte *) gregs >>> + + amd64_linux_gregset32_reg_offset[I386_EAX_REGNUM]; >> >> Need to wrap the expression that spawns two lines in ()s. > > I've made this change. And the other one too. > >>> + >>> + /* Sign extend EAX value to avoid potential syscall restart problems. */ >> >> I'd rather see both implementations use the same comment. Could you >> merge them? > > I started to merge them and then decided to write a more detailed > comment based on the text that I wrote for the commit comment. I > have, for the moment anyway, copied the comment to both locations with > only slight changes which reflect where the comment is located. The > problem with having copies of the same long comment in two or more > places is making sure that if one gets updated, then the rest do too. > It might be better to have one refer to the other. > > I'm thinking that it might be preferable to have something like this > in gdb/gdbserver/linux-x86-low.c: > > /* Sign extend EAX value to avoid potential syscall restart problems. > > See amd64_linux_collect_native_gregset() in gdb/amd64-linux-nat.c > for a detailed explanation. */ I'm OK with that. My concern with different comments in two places is that at some point, like we managed to merge the x86 watchpoints code from gdb and gdbserver under gdb/nat/, we might be able to merge this code as well. And different comments make the job a bit harder for the person doing that, since the person _then_ has to decide what the resulting comment will be like. If we can do it now, it's preferable. A pointer from one end to the other, like you're proposing, works for me too. > Below is a diff showing the new comments. It also includes the > changes which wrap the multi-line expressions in parens. Thanks, that new version of the comment looks great. Pedro Alves