From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 4440 invoked by alias); 4 Jun 2012 01:13:49 -0000 Received: (qmail 4432 invoked by uid 22791); 4 Jun 2012 01:13:48 -0000 X-SWARE-Spam-Status: No, hits=-6.8 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,KHOP_THREADED,RCVD_IN_DNSWL_HI,RCVD_IN_HOSTKARMA_W,SPF_HELO_PASS,TW_RG,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 04 Jun 2012 01:13:31 +0000 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q541DIV9003161 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Sun, 3 Jun 2012 21:13:18 -0400 Received: from valrhona.uglyboxes.com (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id q541DFmf029284 (version=TLSv1/SSLv3 cipher=DHE-RSA-CAMELLIA256-SHA bits=256 verify=NO); Sun, 3 Jun 2012 21:13:17 -0400 Message-ID: <4FCC0BAB.9000402@redhat.com> Date: Mon, 04 Jun 2012 01:13:00 -0000 From: Keith Seitz User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:11.0) Gecko/20120329 Thunderbird/11.0.1 MIME-Version: 1.0 To: Roland Schwingel CC: insight@sourceware.org Subject: Re: [PATCH] libgui: Make it compileable on windows with recent gcc versions and also on win64 References: <4FBF744B.9050305@onevision.com> In-Reply-To: <4FBF744B.9050305@onevision.com> Content-Type: text/plain; charset=ISO-8859-15; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes 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-q2/txt/msg00046.txt.bz2 Thanks for the patch. I only have a (very) minor few nits... On 05/25/2012 05:00 AM, Roland Schwingel wrote: > > diff -ruN libgui_orig/src/subcommand.c libgui/src/subcommand.c > --- libgui_orig/src/subcommand.c 2001-09-09 00:34:47.000000000 +0200 > +++ libgui/src/subcommand.c 2012-02-24 13:53:50.760585300 +0100 > @@ -34,7 +34,7 @@ > table. */ > static int > subcommand_implementation (ClientData cd, Tcl_Interp *interp, > - int argc, char *argv[]) > + int argc,CONST84 char *argv[]) Since this is still our code, I would prefer we stick to GNU as much as possible/feasible. Please watch the spaces after ','. There are several places like this. > { > struct subcommand_clientdata *data = (struct subcommand_clientdata *) cd; > const struct ide_subcommand_table *commands = data->commands; > @@ -90,7 +90,7 @@ > > /* Define a command with subcommands. */ > int > -ide_create_command_with_subcommands (Tcl_Interp *interp, char *name, > +ide_create_command_with_subcommands (Tcl_Interp *interp, const char *name, > const struct ide_subcommand_table *table, > ClientData subdata, > Tcl_CmdDeleteProc *delete) This appears to be > 80 characters and needs line-wrapping. Several places where this happens, too. > diff -ruN libgui_orig/src/tclmsgbox.c libgui/src/tclmsgbox.c > --- libgui_orig/src/tclmsgbox.c 2001-09-09 00:34:47.000000000 +0200 > +++ libgui/src/tclmsgbox.c 2012-02-24 13:33:44.464962700 +0100 > @@ -4,11 +4,11 @@ > > #ifdef _WIN32 > > +#include > + > #include > #include > > -#include > - > /* FIXME: We use some internal Tcl and Tk Windows stuff. */ > #include > > @@ -96,7 +96,7 @@ > class.hInstance = TclWinGetTclInstance(); > class.hbrBackground = NULL; > class.lpszMenuName = NULL; > - class.lpszClassName = "ide_messagebox"; > + class.lpszClassName = TEXT("ide_messagebox"); > class.lpfnWndProc = msgbox_wndproc; > class.hIcon = NULL; > class.hCursor = NULL; GNU wants a space before '(' (when possible). Needs addressing in several places. [I realize this isn't very consistent across libgui, but let's just give it a try.] > diff -ruN libgui_orig/src/tclwinfont.c libgui/src/tclwinfont.c > --- libgui_orig/src/tclwinfont.c 2001-09-09 00:34:47.000000000 +0200 > +++ libgui/src/tclwinfont.c 2012-02-24 13:42:41.393703800 +0100 > @@ -154,7 +154,7 @@ > } > > oldMode = Tcl_SetServiceMode(TCL_SERVICE_ALL); > - if (! ChooseFont (&cf)) > + if (! ChooseFontA (&cf)) > { > DWORD code; Might as well take this opportunity to make this more GNU-compatible. Please use "!ChooseFontA". > diff -ruN libgui_orig/src/tclwinprint.c libgui/src/tclwinprint.c > --- libgui_orig/src/tclwinprint.c 2001-09-09 00:34:47.000000000 +0200 > +++ libgui/src/tclwinprint.c 2012-02-24 13:57:12.703864500 +0100 > diff -ruN libgui_orig/src/tkTable.c libgui/src/tkTable.c > --- libgui_orig/src/tkTable.c 2001-09-09 00:34:47.000000000 +0200 > +++ libgui/src/tkTable.c 2012-02-24 15:25:18.870312200 +0100 > @@ -1041,7 +1037,7 @@ > /* Do the configuration */ > argv = StringifyObjects(objc, objv); > result = Tk_ConfigureWidget(interp, tablePtr->tkwin, tableSpecs, > - objc, argv, (char *) tablePtr, flags); > + objc, (CONST84 char **)argv, (char *) tablePtr, flags); Please use "(CONST84 char **) argv". Another GNU-ism. > ckfree((char *) argv); > if (result != TCL_OK) { > return TCL_ERROR; > diff -ruN libgui_orig/src/tkTable.h libgui/src/tkTable.h > --- libgui_orig/src/tkTable.h 2001-09-09 00:34:47.000000000 +0200 > +++ libgui/src/tkTable.h 2012-03-22 15:50:05.366470500 +0100 > @@ -15,9 +15,26 @@ > #ifndef_TKTABLE_H_ > #define_TKTABLE_H_ > > +#ifdef WIN32 > +# define WIN32_LEAN_AND_MEAN > +# include > +# undef WIN32_LEAN_AND_MEAN > +/* VC++ has an entry point called DllMain instead of DllEntryPoint */ > +# if defined(_MSC_VER) > +# define DllEntryPoint DllMain > +# endif > +#endif I wonder if this will conflict with Cygwin... Honestly, I don't know. I'm still having big problems with Cygwin builds. > @@ -631,6 +638,31 @@ > (rowPtr), (colPtr)) > > /* > + * Macros used to cast between pointers and integers (e.g. when storing an int > + * in ClientData), on 64-bit architectures they avoid gcc warning about "cast > + * to/from pointer from/to integer of different size". > + */ > + > +#if !defined(INT2PTR)&& !defined(PTR2INT) > +# if defined(HAVE_INTPTR_T) || defined(intptr_t) > +# define INT2PTR(p) ((void *)(intptr_t)(p)) > +# define PTR2INT(p) ((int)(intptr_t)(p)) > +# else > +# define INT2PTR(p) ((void *)(p)) > +# define PTR2INT(p) ((int)(p)) > +# endif > +#endif > +#if !defined(UINT2PTR)&& !defined(PTR2UINT) > +# if defined(HAVE_UINTPTR_T) || defined(uintptr_t) > +# define UINT2PTR(p) ((void *)(uintptr_t)(p)) > +# define PTR2UINT(p) ((unsigned int)(uintptr_t)(p)) > +# else > +# define UINT2PTR(p) ((void *)(p)) > +# define PTR2UINT(p) ((unsigned int)(p)) > +# endif > +#endif I'm not going to force this file to the GNU Coding Standard. Just please be consistent with the surrounding code. For example, "!defined(INT2PTR) && !defined(PTR2INT)" seems more natural. > diff -ruN libgui_orig/src/tkWinPrintText.c libgui/src/tkWinPrintText.c > --- libgui_orig/src/tkWinPrintText.c 2001-09-09 00:34:48.000000000 +0200 > +++ libgui/src/tkWinPrintText.c 2012-03-22 13:56:42.037502500 +0100 > +#if (TCL_MAJOR_VERSION>= 8)&& (TCL_MINOR_VERSION>= 5) > + numLines = TkBTreeNumLines(textPtr->sharedTextPtr->tree,textPtr); > + TkTextMakeByteIndex(textPtr->sharedTextPtr->tree, textPtr, 0, 0,&first); > + TkTextMakeByteIndex(textPtr->sharedTextPtr->tree, textPtr, numLines, 100,&last); > + TkTextChanged(textPtr->sharedTextPtr, textPtr,&first,&last); > +#elif (TCL_MAJOR_VERSION>= 8)&& (TCL_MINOR_VERSION>= 1) > numLines = TkBTreeNumLines(textPtr->tree); > -#if (TCL_MAJOR_VERSION>= 8)&& (TCL_MINOR_VERSION>= 1) > TkTextMakeByteIndex(textPtr->tree, 0, 0,&first); > TkTextMakeByteIndex(textPtr->tree, numLines, 100,&last); > + TkTextChanged(textPtr,&first,&last); > #else > + numLines = TkBTreeNumLines(textPtr->tree); > TkTextMakeIndex(textPtr->tree, 0, 0,&first); > TkTextMakeIndex(textPtr->tree, numLines, 100,&last); > -#endif > TkTextChanged(textPtr,&first,&last); > - > +#endif > /* > * Set the display info flag to out-of-date. > */ I'd like to see spaces before/after "&&", "||", ">", "<", ">=", etc. This happens in a few places. Wow, that was one major patch. In the end, I must rely on you and any kind Windows/MinGW-savvy readers to verify the Windows bits. I compiled it all on a Windows VM that I have, but I don't regularly use that platform for any serious work. Ok with those changes. I apologize for the delay in getting this reviewed. It's been anything but timely. Thanks a *bunch* for your patience! Keith