From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 22256 invoked by alias); 2 Jan 2010 08:29:11 -0000 Received: (qmail 22247 invoked by uid 22791); 2 Jan 2010 08:29:10 -0000 X-SWARE-Spam-Status: No, hits=1.0 required=5.0 tests=AWL,BAYES_00 X-Spam-Check-By: sourceware.org Received: from mail.antivirus.flexwebhosting.nl (HELO mail.antivirus.flexwebhosting.nl) (85.92.140.50) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sat, 02 Jan 2010 08:29:05 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by mail.antivirus.flexwebhosting.nl (Postfix) with ESMTP id A4B701218380; Sat, 2 Jan 2010 09:29:01 +0100 (CET) Received: from mail.antivirus.flexwebhosting.nl ([127.0.0.1]) by localhost (mail.antivirus.flexwebhosting.nl [127.0.0.1]) (amavisd-new, port 10024) with LMTP id 72aK4pszq2g2; Sat, 2 Jan 2010 09:28:56 +0100 (CET) Received: from srv4086.flexwebhosting.nl (unknown [89.18.179.86]) by mail.antivirus.flexwebhosting.nl (Postfix) with ESMTP id 1DC0C1218B58; Sat, 2 Jan 2010 09:28:54 +0100 (CET) Received: from ip82-139-82-108.lijbrandt.net ([82.139.82.108] helo=[192.168.2.132]) by srv4086.flexwebhosting.nl with esmtpa (Exim 4.67) (envelope-from ) id 1NQzLw-000270-Ti; Sat, 02 Jan 2010 09:28:53 +0100 Message-ID: <4B3F03C4.1040104@cyberfiber.org> Date: Sat, 02 Jan 2010 08:29:00 -0000 From: Michael User-Agent: Thunderbird 2.0.0.23 (X11/20090817) MIME-Version: 1.0 To: Joel Brobecker , gdb-patches@sourceware.org Subject: Re: gdb-patch mailing list References: <4B3E4DC6.7020901@cyberfiber.org> <20100102043704.GR548@adacore.com> In-Reply-To: <20100102043704.GR548@adacore.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit 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-01/txt/msg00023.txt.bz2 Joel, Ok, I'll have a look at it... I don't have any other thing to do anyway. In the mean time you could perhaps try to find out why my email doesn't appear on the mailing list anymore. Michael. Joel Brobecker wrote: > Michael, > > The gdb-patches mailing list should work. Can you try using it for > the next iteration? > > Again, please state the intent of the patch. What are you fixing? > I can often guess from the patch itself, but sometimes I get it wrong. > A good way of presenting your change, for instance, is to show the debugger > output before and after your patch, > > I am realizing that perhaps you don't know how to read the contents > of patch files? For instance: > > >> diff --git a/gdb/ChangeLog b/gdb/ChangeLog >> index 753fbb0..bc71679 100644 >> --- a/gdb/ChangeLog >> +++ b/gdb/ChangeLog >> > > This bit of the patch says that you are modifying the ChangeLog. > I mentioned in a previous message that it's simpler for you to send > the entry as inlined text inside the body of your email, rather than > as a patch, but I don't mind. However, looking further into the patch, > it says that you are proposing the following modifications to this file: > > > >> - Add new tracepoint action teval. >> - * tracepoint.c (teval_pseudocommand): New function. >> - (validate_actionline): Add teval action case. >> - (encode_actions): Ditto. >> - (_initialize_tracepoint): Define teval pseudocommand. >> - * NEWS: Mention teval. >> > [...] > > The leading minus '-' signs mean that you propose to remove this entry > (and many other entries). I don't think you really meant that. > > I couldn't locate any change that seemed to add an entry, but maybe > I missed it, since the diff was so large and mostly irrelevant. > > >> -/* Print the status word STATUS. */ >> - >> -static void >> -print_i387_status_word (unsigned int status, struct ui_file *file) >> +/* print the status word */ >> +static void print_i387_status_word (unsigned int status, struct ui_file *file) >> > > This part of the patch says that: > (1) You are replacing the following comment: > /* Print the status word STATUS. */ > with: > /* print the status word */ > (2) You are removing the newline between "static void" and > the function name "print_i387_status_word" > > I don't think that (1) was really intended, was it? Your version > brings no additional information, but at the same time no longer > follow the GNU coding style: Comments should be English sentences > starting a capital letter and ending with a dot. Not also the two > spaces after the dot, which is mandated by the GNU Coding Style. > > >> - fprintf_filtered (file, "Status Word: %s", >> - hex_string_custom (status, 4)); >> + fprintf_filtered (file, "status word : %s\n", hex_string_custom(status, 4)); >> > > You made a formatting change which is incorrect: The code should go no > further than column 78-80. Please leave the call to hex_string_custom > on the next line where it was. Make sure, when you send a new patch, > that the diff does not show any difference on this line, particularly > differences in spaces. > > >> + fprintf_filtered (file, "%s ", (status & 0x0020) ? "PE" : " "); /* precision */ >> > > I think that these added comments are superfluous - any developer working > on that code should have a basic knowledge of this FPU and thus know > what these abbreviations mean. I do not mind them so much, except that > they cause the line to exceed 80 chars, or be sufficient close to it, > that these comments should be on a line of their own. > >