From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 111875 invoked by alias); 29 Dec 2019 20:32:59 -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 111119 invoked by uid 89); 29 Dec 2019 20:32:59 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-11.4 required=5.0 tests=AWL,BAYES_00,KAM_NUMSUBJECT,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=no version=3.3.1 spammy=UD:m, nov, HX-Google-Smtp-Source:APXvYqym, covers X-HELO: mail-wm1-f51.google.com Received: from mail-wm1-f51.google.com (HELO mail-wm1-f51.google.com) (209.85.128.51) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sun, 29 Dec 2019 20:32:57 +0000 Received: by mail-wm1-f51.google.com with SMTP id p17so12757500wmb.0 for ; Sun, 29 Dec 2019 12:32:57 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=embecosm.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=QFAg3w5cFOfSg1TXqf4gGV2CqRetZm649Z4/fgXraXA=; b=AZPLjKiBWpFqBOEW3JpSIn4NIugl0CFWNeEg3Hv7O6zxmK0/D+snP75v3rVKRmv9Dj mubA5u1vQI019jHcUEqwoOaMnlLwlS7mjZ7oABZi3gsIyC3BxLDjSQLWKy3xQsARYNL/ Zj09rDcB2TRAFgh336dGkNEeX9uHbzp2X86bg4vJ4q1sUJB9iDSVA09pvg/o7xNFMj2I EyfJm0xS8K09YHDi13ebFQkn98iuXpIyrk/DEf6Mq8+b2iRKvJyUL2jeXIZkHdlV3/b/ LPX0xTM644159dEFI7uyV0KgDpltHtgMGriP26zxyqq1FRDa5qE/hYBju8wEHlNnbL/4 wsQg== Return-Path: Received: from localhost (host86-186-80-236.range86-186.btcentralplus.com. [86.186.80.236]) by smtp.gmail.com with ESMTPSA id u24sm18405821wml.10.2019.12.29.12.32.54 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Sun, 29 Dec 2019 12:32:54 -0800 (PST) Date: Sun, 29 Dec 2019 20:32:00 -0000 From: Andrew Burgess To: Bernd Edlinger Cc: Simon Marchi , "gdb-patches@sourceware.org" Subject: Re: [PATCHv2] Fix setting breakpoints or stepping on line 65535 Message-ID: <20191229203253.GP3865@embecosm.com> References: <20191201220818.GI3410@embecosm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Fortune: I must follow the people. Am I not their leader? -Benjamin Disraeli X-Editor: GNU Emacs [ http://www.gnu.org/software/emacs ] User-Agent: Mutt/1.9.2 (2017-12-15) X-IsSubscribed: yes X-SW-Source: 2019-12/txt/msg01063.txt.bz2 * Simon Marchi [2019-12-29 14:45:13 -0500]: > On 2019-12-02 1:04 p.m., Bernd Edlinger wrote: > > Hi Andrew, > > > > On 12/1/19 11:08 PM, Andrew Burgess wrote: > >> * Bernd Edlinger [2019-11-24 11:54:23 +0000]: > >> > >>> Hi, > >>> > >>> this removes code that is present from the very first git revisison > >>> 7b4ac7e1ed2c4616bce56d1760807798be87ac9e from 1988. It was in the > >>> gdb/dbxread.c at the time (and makes more sense for dbx line info format > >>> since line numbers are 16-bit entities in that debug format and debugging > >>> files with more than 65535 lines would not work anyway) but moved from > >>> there to gdb/buildsym.c which is used for dwarf line info as well, and > >>> excluding an arbitrary line number does certainly not make sense nowadays. > >>> > >>> > >>> Thanks > >>> Bernd. > >> > >>> From f202ae765b72ad6d17600eb661993a63191309f7 Mon Sep 17 00:00:00 2001 > >>> From: Bernd Edlinger > >>> Date: Sat, 23 Nov 2019 07:37:26 +0100 > >>> Subject: [PATCH 1/2] Fix setting breakpoints or stepping on line 65535 > >>> > >> > >> Bernd, > >> > >> Thanks for looking into this, and especially thanks for adding a test! > >> > >> Normally you should include the git commit message and ChangeLog along > >> with your patch submission so that these can be reviewed too. 'git > >> format-patch' and 'git send-email' can be useful for this, if you can > >> get them setup. > >> > > > > Okay, I added changelog messages for gdb/ChangeLog and gdb/testsuite/ChangeLog, > > and improved the commit messages. > > I hope you can handle the format-patch files as attachments. > > > >> Given the age of the code you're removing I think this change sounds > >> reasonable. I assume there's no test that covers why this code should > >> be there, so you see no regressions with this code removed? > >> > > > > No, there were no regressions (last full test on 24-11-19). > > > >> I have a couple of minor issues with the test. If you address those > >> and repost with commit message and ChangeLog this can be approved. > >> > > > > Fixed the test. > > > > Thanks a lot for your review. > > Is it OK for trunk? > > Thanks Bernd, this is OK. For the future, I think it would be reasonable to put the > fix and the test in the same patch. It would also help somebody wondering why we have > such a weird test trace it back to the fix, and the explanation you have put in the first > patch. But it's also fine like this. I place more value on having tests and changes in the same commit - I find it more useful the other way around, when I find some code I don't understand and I figure out which commit it came from, having the tests in the same commit means it is easy to know "this test" should check "this code". If the test and changes are separate I have to do more work. Boo! For this reason, if you've not already pushed this patch, could you collapse the test and change into a single commit please before pushing. Thanks, Andrew