From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 18031 invoked by alias); 27 Mar 2012 07:13:48 -0000 Received: (qmail 18017 invoked by uid 22791); 27 Mar 2012 07:13:46 -0000 X-SWARE-Spam-Status: No, hits=-1.5 required=5.0 tests=AWL,BAYES_00,TW_BT,TW_DB X-Spam-Check-By: sourceware.org Received: from outdoor.onevision.de (HELO outdoor.onevision.de) (212.77.172.51) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 27 Mar 2012 07:13:33 +0000 Received: from sanders.onevision.de (moonrace [212.77.172.62]) by outdoor.onevision.de (8.14.3/8.13.7/ROSCH/DDB) with ESMTP id q2R7DMrt003851; Tue, 27 Mar 2012 09:13:27 +0200 Received: from [192.168.5.32] ([192.168.5.32]) by sanders.onevision.de (Lotus Domino Release 8.5.1FP3) with ESMTP id 2012032709131783-2533 ; Tue, 27 Mar 2012 09:13:17 +0200 Message-ID: <4F716889.2080603@onevision.com> Date: Tue, 27 Mar 2012 07:13:00 -0000 From: Roland Schwingel User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:10.0.2) Gecko/20120216 Thunderbird/10.0.2 MIME-Version: 1.0 To: insight@sourceware.org, keiths@redhat.com Subject: Re: [PATCH] Remove deprecated access to tcl internal variables Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset=ISO-8859-15; format=flowed Mailing-List: contact insight-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: insight-owner@sourceware.org X-SW-Source: 2012-q1/txt/msg00037.txt.bz2 Hi Keith... Keith Seitz wrote on 23.03.2012 23:25:06: > I noticed this a while back, too.. Some small nits. > > In the future, could you please > add "-p" option to diff? I have "diff -upN" set in my .cvsrc. It gives > me a little more context when reading the patch. Sure no problem. > On 03/19/2012 04:45 AM, Roland Schwingel wrote: > > --- gdbtk_orig/generic/gdbtk.c 2012-03-19 11:21:15.542232400 +0100 > > +++ gdbtk/generic/gdbtk.c 2012-03-19 11:23:12.099170600 +0100 > > @@ -494,17 +494,17 @@ > > make_final_cleanup (gdbtk_cleanup, NULL); > > > > if (Tcl_Init (gdbtk_interp) != TCL_OK) > > - error ("Tcl_Init failed: %s", gdbtk_interp->result); > > + error ("Tcl_Init failed: %s", Tcl_GetStringResult(gdbtk_interp)); > > Watch the spaces in between function names and open parenthesis. This is > a GNU convention (which we follow). That should be "Tcl_GetStringResult > (gdbtk_interp)". > > There are several places where this occurs throughout the patches (and > subsequent ones). Sorry... Sometimes it is hard to switch between the styles. I surely want to follow the GCS when submitting patches. Our company style is just (in some areas) vice versa. Shall I resubmit my patch(es)? If you give the ok to my patch(es) I can also do these final reformatting during commit. > > tcl_compat_gdbtk-hooks.c.patch > > > > > > --- gdbtk_orig/generic/gdbtk-hooks.c 2012-01-03 13:26:56.000000000 +0100 > > +++ gdbtk/generic/gdbtk-hooks.c 2012-03-05 11:47:03.340565000 +0100 > > @@ -254,17 +254,22 @@ > > { > > report_error (); > > actual_len = 0; > > + buf[0] = '\0'; > > + return 0; > > This hunk is not mentioned in the ChangeLog. > > I don't understand this. If we return 0 here, that means that no bytes > in the buffer were filled-in, yet we are actually filling in one byte > anyway. Is there a problem this was designed to solve? Is the return > value not being checked somewhere or buf assumed to have at least the > string terminal in it? I did not change behaviour here Keith. gdbtk_read() behaves the same as before it just takes a small shortcut in case gdbtk_console_read has failed (for some reasons). In the unpatched code actual_len is set to 0 in this case. Execution continues and later on also results in buf[actual_len] = '\0'; return actual_len; where actual_len is 0. So same behaviour. I believe this was done to bring the buffer in a defined (empty) state. I did it that way to make the code easier to read as I am calling Tcl_GetStringResult() and I just want to call it just once and avoid additional ifs for handling the error case. Maybe this was too obvious for me so I did not mention it in the changelog and it does not change behaviour of the function. Please tell me how to go on with the patches. Resubmit (with -p flag on diff) and a second round on GCS? Roland