[Simh] [PATCH] fixing ibm1130

[Peter Lund]

The current version of the IBM 1130 emulator breaks the build under
cygwin.

It does so because it tries to define two functions with names that are
reserved by C99 (and C89/90) because they begin with 'str' and a
lower-case letter.

Problem:
 The simulator tries to parse a few simple things ("off", "!BREAK",
"XR") in a case-insensitive manner.  Standard C makes that hard by not
providing case-insensitive string comparison functions (it only has
strcmp()/strncmp()).  POSIX has strcasecmp()/strncasecmp().  DOS/Windows
traditionally have several case-insensitive string comparison functions:
stricmp()/strcmpi()/strnicmp()/strncmpi() + a few more that are only
supported by a few compilers.  Those functions tend to have many
synonyms under DOS/Windows.  Unfortunately, strcasecmp()/strncasecmp()
seems not to be one of them :(

The simulator uses strnicmp() and strcmpi(), which are provided under
DOS and Windows and it has its definition of the two that it tries to
enable on other platforms.

This would work almost everywhere, if the test it used to see whether it
should use its own definitions worked.

It doesn't.

It only tests for the presence of the macro WIN32, which ISN'T defined
under cygwin.  It is also not defined by any of the pure DOS compilers I
know of.  In other words, it's a platform test and platform tests are
VERY hard to get right when you write them and VERY hard to keep up to
date.  Feature tests are much better.

If you try to override the definition of existing library functions you
can almost always make it work if you really know what you are doing but
the solution is often tailored to a specific compiler/library
combination.

In this particular case it breaks because the header files on Windows
defines a macro with a name identical to one of the function names that
expands to one of the other synonyms for the function.  This confuses
the compiler a bit...

It could most likely be fixed in this particular case by #undef'ing the
name first.  But who knows what might go wrong on other platforms?


The best solution seems to me to be absolutely standards compliant --
and then forgo the option of using a native function entirely.

This patch does that.

Tested with gcc 4.1.2 on Ubuntu Feisty Fawn and gcc 3.4.4 from a very
recent cygwin on Windows XP (both under bash and cmd.exe).

Please apply.

-Peter


 ibm1130_cpu.c  |    2 +-
 ibm1130_cr.c   |    4 ++--
 ibm1130_defs.h |    6 ++----
 ibm1130_disk.c |    2 +-
 ibm1130_sys.c  |    9 +++------
 5 files changed, 9 insertions(+), 14 deletions(-)


(The patch may look ugly in your mail program due to the carriage
returns at the end -- I'll resend the patch as an attachment in the next
mail so you can be absolutely sure you get the right patch.)

diff -r 6034dd526ac5 Ibm1130/ibm1130_cpu.c
--- a/Ibm1130/ibm1130_cpu.c	Tue Oct 02 23:23:43 2007 +0200
+++ b/Ibm1130/ibm1130_cpu.c	Wed Oct 03 00:01:36 2007 +0200
@@ -1373,7 +1373,7 @@ t_stat cpu_set_type (UNIT *uptr, int32 v
 	is_1800 = (value & UNIT_1800) != 0;					/* set is_1800 mode flag */
 
 	for (r = cpu_reg; r->name != NULL; r++) {			/* unhide or hide 1800-specific registers & state */
-		if (strnicmp(r->name, "XR", 2) == 0) {
+		if (our_strnicmp(r->name, "XR", 2) == 0) {
 			if (value & UNIT_1800)
 				CLRBIT(r->flags, REG_HIDDEN|REG_RO);
 			else
diff -r 6034dd526ac5 Ibm1130/ibm1130_cr.c
--- a/Ibm1130/ibm1130_cr.c	Tue Oct 02 23:23:43 2007 +0200
+++ b/Ibm1130/ibm1130_cr.c	Wed Oct 03 00:02:02 2007 +0200
@@ -1284,7 +1284,7 @@ static t_bool nextdeck (void)
 		if (*buf == '#' || *buf == '*' || *buf == ';')
 			continue;						/* comment */
 
-		if (strnicmp(buf, "!BREAK", 6) == 0) {	/* stop the simulation */
+		if (our_strnicmp(buf, "!BREAK", 6) == 0) {	/* stop the simulation */
 			break_simulation(STOP_DECK_BREAK);
 			continue;
 		}
@@ -1350,7 +1350,7 @@ static t_bool nextdeck (void)
 				fpos = ftell(deckfile);
 				if (fgets(buf, sizeof(buf), deckfile) == NULL)
 					break;							/* oops, end of file */
-				if (buf[0] != '!' || strnicmp(buf, "!BREAK", 6) == 0)
+				if (buf[0] != '!' || our_strnicmp(buf, "!BREAK", 6) == 0)
 					break;
 				alltrim(buf);
 			}
diff -r 6034dd526ac5 Ibm1130/ibm1130_defs.h
--- a/Ibm1130/ibm1130_defs.h	Tue Oct 02 23:23:43 2007 +0200
+++ b/Ibm1130/ibm1130_defs.h	Wed Oct 03 00:02:30 2007 +0200
@@ -26,10 +26,8 @@
 #define MIN(a,b)  (((a) <= (b)) ? (a) : (b))
 #define MAX(a,b)  (((a) >= (b)) ? (a) : (b))
 
-#ifndef _WIN32
-   int strnicmp (const char *a, const char *b, int n);
-   int strcmpi  (const char *a, const char *b);
-#endif
+extern int our_strnicmp (const char *a, const char *b, int n);
+extern int our_strcmpi  (const char *a, const char *b);
 
 /* #define GUI_SUPPORT		uncomment to compile the GUI extensions. It's defined in the windows ibm1130.mak makefile */
 
diff -r 6034dd526ac5 Ibm1130/ibm1130_disk.c
--- a/Ibm1130/ibm1130_disk.c	Tue Oct 02 23:23:43 2007 +0200
+++ b/Ibm1130/ibm1130_disk.c	Wed Oct 03 00:03:17 2007 +0200
@@ -648,7 +648,7 @@ static t_stat phdebug_cmd (int flag, cha
 {
 	int val1, val2;
 
-	if (strcmpi(ptr, "off") == 0)
+	if (our_strcmpi(ptr, "off") == 0)
 		phdebug_lo = phdebug_hi = -1;
 	else {
 		switch(sscanf(ptr, "%x%x", &val1, &val2)) {
diff -r 6034dd526ac5 Ibm1130/ibm1130_sys.c
--- a/Ibm1130/ibm1130_sys.c	Tue Oct 02 23:23:43 2007 +0200
+++ b/Ibm1130/ibm1130_sys.c	Wed Oct 03 00:02:42 2007 +0200
@@ -453,9 +453,7 @@ t_stat parse_sym (char *cptr, t_addr add
 	return SCPE_ARG;
 }
 
-#ifndef _WIN32
-
-int strnicmp (const char *a, const char *b, int n)
+int our_strnicmp (const char *a, const char *b, int n)
 {
 	int ca, cb;
 
@@ -480,7 +478,7 @@ int strnicmp (const char *a, const char 
 	}
 }
 
-int strcmpi (const char *a, const char *b)
+int our_strcmpi (const char *a, const char *b)
 {
 	int ca, cb;
 
@@ -501,5 +499,4 @@ int strcmpi (const char *a, const char *
 		a++, b++;
 	}
 }
-
-#endif
+