From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 23699 invoked by alias); 5 May 2010 22:39:19 -0000 Received: (qmail 23689 invoked by uid 22791); 5 May 2010 22:39:18 -0000 X-SWARE-Spam-Status: No, hits=-2.2 required=5.0 tests=AWL,BAYES_00,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (38.113.113.100) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 05 May 2010 22:39:15 +0000 Received: (qmail 6804 invoked from network); 5 May 2010 22:39:13 -0000 Received: from unknown (HELO orlando.localnet) (pedro@127.0.0.2) by mail.codesourcery.com with ESMTPA; 5 May 2010 22:39:13 -0000 From: Pedro Alves To: Michael Snyder Subject: Re: [ob] remote.c, eliminate unused variables Date: Wed, 05 May 2010 22:39:00 -0000 User-Agent: KMail/1.12.2 (Linux/2.6.31-20-generic; KDE/4.3.2; x86_64; ; ) Cc: "gdb-patches@sourceware.org" References: <4BE1D93F.8000309@vmware.com> <201005052241.14753.pedro@codesourcery.com> <4BE1EF9C.8070807@vmware.com> In-Reply-To: <4BE1EF9C.8070807@vmware.com> MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-15" Content-Transfer-Encoding: 7bit Message-Id: <201005052339.07578.pedro@codesourcery.com> X-IsSubscribed: yes 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 X-SW-Source: 2010-05/txt/msg00137.txt.bz2 On Wednesday 05 May 2010 23:22:20, Michael Snyder wrote: > >> Can't guarantee it, no. > >> I'm making sure the semantics isn't changed, but I can't always > >> be sure that the original semantics was right. > > > > Well, then I'll ask please, don't "fix" more things like this, > > and surely don't call it obvious. You're removing a warning for > > the sake of it. > > No, I'm attempting to make the code easier to understand by > removing dead code and variables. Since this warning is > turned off, I'm not even reducing the number of warnings. You're hiding the bug for whoever wants to catch these bugs by enabling the warning in its local tree. That's what I mean: you must be enabling the warning explicitly to see these; if one doesn't want to check if the warnings are pointing at something wrong, then one just shouldn't enable the warning in the first place. > > A warning is useful as a hint at something > > wrong with the code; there may be something genuinely wrong > > with it. Removing it blindly removes the useful hint. > > There's no hint if the warning is turned off. If I hadn't > touched it and you hadn't reviewed my change, it would have > remained undiscovered indefinitely. I'll see it the same way you must be seeing it. By enabling the warning on my local build. > So let's fix it, shall we? I'll post a separate patch for you to review. To be clear, I'm very much not interested in reviewing these kind of patches. That would mean doing about the same work the person writing the patch is already doing. What I'm intersted in, is making sure that whenever these patches go in, it was made sure the variables weren't being unused because we forgot to use them, instead of just deleting them. If there's any doubt that's the case, then the patch isn't obvious, and it should be posted for comments. > > If you > > want to be bothered to look at the code to see if there's > > something else genuinely wrong, then please, don't change it. > > That's not fair, I did "bother" to look at the code. > One got by me, that's all. Thanks for catching it. Okay, thanks. I misunderstood perhaps, your reply seemed to imply you didn't ("Are you making sure" -> "Can't guarantee it, no"). -- Pedro Alves