From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 25593 invoked by alias); 2 Jan 2010 08:48:13 -0000 Received: (qmail 25584 invoked by uid 22791); 2 Jan 2010 08:48:12 -0000 X-SWARE-Spam-Status: No, hits=0.6 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:48:08 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by mail.antivirus.flexwebhosting.nl (Postfix) with ESMTP id CE1FC11F02A1; Sat, 2 Jan 2010 09:48:04 +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 qxJHf2A+ALHI; Sat, 2 Jan 2010 09:47:59 +0100 (CET) Received: from srv4086.flexwebhosting.nl (unknown [89.18.179.86]) by mail.antivirus.flexwebhosting.nl (Postfix) with ESMTP id 44A9A11F028A; Sat, 2 Jan 2010 09:47:57 +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 1NQzeO-000319-8E; Sat, 02 Jan 2010 09:47:56 +0100 Message-ID: <4B3F083C.2080300@cyberfiber.org> Date: Sat, 02 Jan 2010 08:48: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> <4B3F03C4.1040104@cyberfiber.org> In-Reply-To: <4B3F03C4.1040104@cyberfiber.org> 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/msg00024.txt.bz2 Michael wrote: > 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. this is not mine, I think you are in error?? >> >> 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. >> >> > >