This is the mail archive of the insight@sourceware.org mailing list for the Insight project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] libgui: Make it compileable on windows with recent gcc versions and also on win64


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<windows.h>
+
  #include<tcl.h>
  #include<tk.h>

-#include<windows.h>
-
  /* FIXME: We use some internal Tcl and Tk Windows stuff.  */
  #include<tkWinInt.h>

@@ -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<windows.h>
+#   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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]