sim/arm/armos.c: IsTTY [PATCH]

sim/arm/armos.c: IsTTY [PATCH]

[Shaun Jackman]

[-- Attachment #1: Type: text/plain, Size: 3863 bytes --]

This patch adds support for the ARM IsTTY, Remove, and Rename SWI calls.

Please cc me in your reply. Cheers,
Shaun

2005-08-15  Shaun Jackman  <sjackman@gmail.com>

	* sim/arm/armos.c (unlink): Remove this macro. It is unused
	in this file and conflicts with sim_callback->unlink.
	(SWIremove): New function.
	(SWIrename): Ditto.
	(ARMul_OSHandleSWI): Handle the RDP calls SWI_IsTTY,
	SWI_Remove, and SWI_Rename, as well as the RDI calls
	AngelSWI_Reason_IsTTY, AngelSWI_Reason_Remove, and
	AngelSWI_Reason_Rename.

Index: sim/arm/armos.c
===================================================================
RCS file: /cvs/src/src/sim/arm/armos.c,v
retrieving revision 1.22
diff -u -r1.22 armos.c
--- sim/arm/armos.c	12 May 2005 07:36:58 -0000	1.22
+++ sim/arm/armos.c	15 Aug 2005 18:41:51 -0000
@@ -34,10 +34,6 @@
 #define TARGET_O_BINARY 0
 #endif
 
-#ifdef __STDC__
-#define unlink(s) remove(s)
-#endif
-
 #ifdef HAVE_UNISTD_H
 #include <unistd.h>		/* For SEEK_SET etc.  */
 #endif
@@ -403,6 +399,36 @@
   OSptr->ErrorNo = sim_callback->get_errno (sim_callback);
 }
 
+static void
+SWIremove (ARMul_State * state, ARMword path)
+{
+  struct OSblock *OSptr = (struct OSblock *) state->OSptr;
+  char dummy[2000];
+  int i;
+
+  for (i = 0; (dummy[i] = ARMul_SafeReadByte (state, path + i)); i++)
+    ;
+
+  state->Reg[0] = sim_callback->unlink (sim_callback, dummy);
+  OSptr->ErrorNo = sim_callback->get_errno (sim_callback);
+}
+
+static void
+SWIrename (ARMul_State * state, ARMword old, ARMword new)
+{
+  struct OSblock *OSptr = (struct OSblock *) state->OSptr;
+  char olddummy[2000], newdummy[2000];
+  int i;
+
+  for (i = 0; (olddummy[i] = ARMul_SafeReadByte (state, old + i)); i++)
+    ;
+  for (i = 0; (newdummy[i] = ARMul_SafeReadByte (state, new + i)); i++)
+    ;
+
+  state->Reg[0] = sim_callback->rename (sim_callback, olddummy, newdummy);
+  OSptr->ErrorNo = sim_callback->get_errno (sim_callback);
+}
+
 /* The emulator calls this routine when a SWI instruction is encuntered.
    The parameter passed is the SWI number (lower 24 bits of the
instruction).  */
 
@@ -544,6 +570,30 @@
       state->Emulate = FALSE;
       break;
 
+    case SWI_Remove:
+      if (swi_mask & SWI_MASK_DEMON)
+	SWIremove (state, state->Reg[0]);
+      else
+	unhandled = TRUE;
+      break;
+
+    case SWI_Rename:
+      if (swi_mask & SWI_MASK_DEMON)
+	SWIrename (state, state->Reg[0], state->Reg[1]);
+      else
+	unhandled = TRUE;
+      break;
+
+    case SWI_IsTTY:
+      if (swi_mask & SWI_MASK_DEMON)
+	{
+	  state->Reg[0] = sim_callback->isatty (sim_callback, state->Reg[0]);
+	  OSptr->ErrorNo = sim_callback->get_errno (sim_callback);
+	}
+      else
+	unhandled = TRUE;
+      break;
+
       /* Handle Angel SWIs as well as Demon ones.  */
     case AngelSWI_ARM:
     case AngelSWI_Thumb:
@@ -566,10 +616,7 @@
 	  
 	      /* Unimplemented reason codes.  */
 	    case AngelSWI_Reason_ReadC:
-	    case AngelSWI_Reason_IsTTY:
 	    case AngelSWI_Reason_TmpNam:
-	    case AngelSWI_Reason_Remove:
-	    case AngelSWI_Reason_Rename:
 	    case AngelSWI_Reason_System:
 	    case AngelSWI_Reason_EnterSVC:
 	    default:
@@ -684,6 +731,21 @@
 			ARMul_ReadWord (state, addr + 4),
 			ARMul_ReadWord (state, addr + 8));
 	      break;
+
+	    case AngelSWI_Reason_IsTTY:
+	      state->Reg[0] = sim_callback->close (sim_callback,
+						   ARMul_ReadWord (state, addr));
+	      OSptr->ErrorNo = sim_callback->get_errno (sim_callback);
+	      break;
+
+	    case AngelSWI_Reason_Remove:
+	      SWIremove (state,
+			 ARMul_ReadWord (state, addr));
+
+	    case AngelSWI_Reason_Rename:
+	      SWIrename (state,
+			 ARMul_ReadWord (state, addr),
+			 ARMul_ReadWord (state, addr + 4));
 	    }
 	}
       else

[-- Attachment #2: sim-arm-swi.diff --]
[-- Type: text/plain, Size: 3610 bytes --]

2005-08-15  Shaun Jackman  <sjackman@gmail.com>

	* sim/arm/armos.c (unlink): Remove this macro. It is unused
	in this file and conflicts with sim_callback->unlink.
	(SWIremove): New function.
	(SWIrename): Ditto.
	(ARMul_OSHandleSWI): Handle the RDP calls SWI_IsTTY,
	SWI_Remove, and SWI_Rename, as well as the RDI calls
	AngelSWI_Reason_IsTTY, AngelSWI_Reason_Remove, and
	AngelSWI_Reason_Rename.

Index: sim/arm/armos.c
===================================================================
RCS file: /cvs/src/src/sim/arm/armos.c,v
retrieving revision 1.22
diff -u -r1.22 armos.c
--- sim/arm/armos.c	12 May 2005 07:36:58 -0000	1.22
+++ sim/arm/armos.c	15 Aug 2005 18:41:51 -0000
@@ -34,10 +34,6 @@
 #define TARGET_O_BINARY 0
 #endif
 
-#ifdef __STDC__
-#define unlink(s) remove(s)
-#endif
-
 #ifdef HAVE_UNISTD_H
 #include <unistd.h>		/* For SEEK_SET etc.  */
 #endif
@@ -403,6 +399,36 @@
   OSptr->ErrorNo = sim_callback->get_errno (sim_callback);
 }
 
+static void
+SWIremove (ARMul_State * state, ARMword path)
+{
+  struct OSblock *OSptr = (struct OSblock *) state->OSptr;
+  char dummy[2000];
+  int i;
+
+  for (i = 0; (dummy[i] = ARMul_SafeReadByte (state, path + i)); i++)
+    ;
+
+  state->Reg[0] = sim_callback->unlink (sim_callback, dummy);
+  OSptr->ErrorNo = sim_callback->get_errno (sim_callback);
+}
+
+static void
+SWIrename (ARMul_State * state, ARMword old, ARMword new)
+{
+  struct OSblock *OSptr = (struct OSblock *) state->OSptr;
+  char olddummy[2000], newdummy[2000];
+  int i;
+
+  for (i = 0; (olddummy[i] = ARMul_SafeReadByte (state, old + i)); i++)
+    ;
+  for (i = 0; (newdummy[i] = ARMul_SafeReadByte (state, new + i)); i++)
+    ;
+
+  state->Reg[0] = sim_callback->rename (sim_callback, olddummy, newdummy);
+  OSptr->ErrorNo = sim_callback->get_errno (sim_callback);
+}
+
 /* The emulator calls this routine when a SWI instruction is encuntered.
    The parameter passed is the SWI number (lower 24 bits of the instruction).  */
 
@@ -544,6 +570,30 @@
       state->Emulate = FALSE;
       break;
 
+    case SWI_Remove:
+      if (swi_mask & SWI_MASK_DEMON)
+	SWIremove (state, state->Reg[0]);
+      else
+	unhandled = TRUE;
+      break;
+
+    case SWI_Rename:
+      if (swi_mask & SWI_MASK_DEMON)
+	SWIrename (state, state->Reg[0], state->Reg[1]);
+      else
+	unhandled = TRUE;
+      break;
+
+    case SWI_IsTTY:
+      if (swi_mask & SWI_MASK_DEMON)
+	{
+	  state->Reg[0] = sim_callback->isatty (sim_callback, state->Reg[0]);
+	  OSptr->ErrorNo = sim_callback->get_errno (sim_callback);
+	}
+      else
+	unhandled = TRUE;
+      break;
+
       /* Handle Angel SWIs as well as Demon ones.  */
     case AngelSWI_ARM:
     case AngelSWI_Thumb:
@@ -566,10 +616,7 @@
 	  
 	      /* Unimplemented reason codes.  */
 	    case AngelSWI_Reason_ReadC:
-	    case AngelSWI_Reason_IsTTY:
 	    case AngelSWI_Reason_TmpNam:
-	    case AngelSWI_Reason_Remove:
-	    case AngelSWI_Reason_Rename:
 	    case AngelSWI_Reason_System:
 	    case AngelSWI_Reason_EnterSVC:
 	    default:
@@ -684,6 +731,21 @@
 			ARMul_ReadWord (state, addr + 4),
 			ARMul_ReadWord (state, addr + 8));
 	      break;
+
+	    case AngelSWI_Reason_IsTTY:
+	      state->Reg[0] = sim_callback->close (sim_callback,
+						   ARMul_ReadWord (state, addr));
+	      OSptr->ErrorNo = sim_callback->get_errno (sim_callback);
+	      break;
+
+	    case AngelSWI_Reason_Remove:
+	      SWIremove (state,
+			 ARMul_ReadWord (state, addr));
+
+	    case AngelSWI_Reason_Rename:
+	      SWIrename (state,
+			 ARMul_ReadWord (state, addr),
+			 ARMul_ReadWord (state, addr + 4));
 	    }
 	}
       else

Re: sim/arm/armos.c: IsTTY [PATCH]

[Shaun Jackman]

Any comments?

Thanks,
Shaun

2005/8/15, Shaun Jackman <sjackman@gmail.com>:
> This patch adds support for the ARM IsTTY, Remove, and Rename SWI calls.
> 
> Please cc me in your reply. Cheers,
> Shaun
> 
> 2005-08-15  Shaun Jackman  <sjackman@gmail.com>
> 
>         * sim/arm/armos.c (unlink): Remove this macro. It is unused
>         in this file and conflicts with sim_callback->unlink.
>         (SWIremove): New function.
>         (SWIrename): Ditto.
>         (ARMul_OSHandleSWI): Handle the RDP calls SWI_IsTTY,
>         SWI_Remove, and SWI_Rename, as well as the RDI calls
>         AngelSWI_Reason_IsTTY, AngelSWI_Reason_Remove, and
>         AngelSWI_Reason_Rename.

Re: sim/arm/armos.c: IsTTY [PATCH]

[Daniel Jacobowitz]

On Mon, Aug 15, 2005 at 12:04:13PM -0700, Shaun Jackman wrote:
> This patch adds support for the ARM IsTTY, Remove, and Rename SWI calls.
> 
> Please cc me in your reply. Cheers,
> Shaun
> 
> 2005-08-15  Shaun Jackman  <sjackman@gmail.com>
> 
> 	* sim/arm/armos.c (unlink): Remove this macro. It is unused
> 	in this file and conflicts with sim_callback->unlink.
> 	(SWIremove): New function.
> 	(SWIrename): Ditto.
> 	(ARMul_OSHandleSWI): Handle the RDP calls SWI_IsTTY,
> 	SWI_Remove, and SWI_Rename, as well as the RDI calls
> 	AngelSWI_Reason_IsTTY, AngelSWI_Reason_Remove, and
> 	AngelSWI_Reason_Rename.

It looks plausible to me; Richard, any comments?

> +  char dummy[2000];
> +  int i;
> +
> +  for (i = 0; (dummy[i] = ARMul_SafeReadByte (state, path + i)); i++)
> +    ;

Please no buffer overflows be adding.  Also, there's plenty of ways to
write this without the ugly empty loop body...

-- 
Daniel Jacobowitz
CodeSourcery, LLC

Re: sim/arm/armos.c: IsTTY [PATCH]

[Richard Earnshaw]

On Tue, 2005-08-30 at 03:37, Daniel Jacobowitz wrote:
> On Mon, Aug 15, 2005 at 12:04:13PM -0700, Shaun Jackman wrote:
> > This patch adds support for the ARM IsTTY, Remove, and Rename SWI calls.
> > 
> > Please cc me in your reply. Cheers,
> > Shaun
> > 
> > 2005-08-15  Shaun Jackman  <sjackman@gmail.com>
> > 
> > 	* sim/arm/armos.c (unlink): Remove this macro. It is unused
> > 	in this file and conflicts with sim_callback->unlink.
> > 	(SWIremove): New function.
> > 	(SWIrename): Ditto.
> > 	(ARMul_OSHandleSWI): Handle the RDP calls SWI_IsTTY,
> > 	SWI_Remove, and SWI_Rename, as well as the RDI calls
> > 	AngelSWI_Reason_IsTTY, AngelSWI_Reason_Remove, and
> > 	AngelSWI_Reason_Rename.
> 
> It looks plausible to me; Richard, any comments?
> 

I've no objections, these are part of the standard SWI set, so I see no
real reason not to implement them if we can.

> > +  char dummy[2000];
> > +  int i;
> > +
> > +  for (i = 0; (dummy[i] = ARMul_SafeReadByte (state, path + i)); i++)
> > +    ;
> 
> Please no buffer overflows be adding.  Also, there's plenty of ways to
> write this without the ugly empty loop body...

Agreed.

R.

Re: sim/arm/armos.c: IsTTY [PATCH]

[Shaun Jackman]

[-- Attachment #1: Type: text/plain, Size: 5407 bytes --]

2005/8/29, Daniel Jacobowitz <drow@false.org>:
> Please no buffer overflows be adding.  Also, there's plenty of ways to
> write this without the ugly empty loop body...

I fixed the potential buffer overflow and cleaned up the loop coding
style. Thanks for the tips.

Cheers,
Shaun

2005-09-07  Shaun Jackman  <sjackman@gmail.com>

	* sim/arm/armos.c (unlink): Remove this macro. It is unused
	in this file and conflicts with sim_callback->unlink.
	(SWIopen): Fix a potential buffer overflow.
	(SWIremove): New function.
	(SWIrename): Ditto.
	(ARMul_OSHandleSWI): Handle the RDP calls SWI_IsTTY,
	SWI_Remove, and SWI_Rename, as well as the RDI calls
	AngelSWI_Reason_IsTTY, AngelSWI_Reason_Remove, and
	AngelSWI_Reason_Rename.

Index: sim/arm/armos.c
===================================================================
RCS file: /cvs/src/src/sim/arm/armos.c,v
retrieving revision 1.22
diff -u -r1.22 armos.c
--- sim/arm/armos.c	12 May 2005 07:36:58 -0000	1.22
+++ sim/arm/armos.c	7 Sep 2005 16:45:27 -0000
@@ -27,6 +27,7 @@
 
  #include <time.h>
  #include <errno.h>
+#include <limits.h>
  #include <string.h>
  #include "targ-vals.h"
 
@@ -34,10 +35,6 @@
  #define TARGET_O_BINARY 0
  #endif
 
-#ifdef __STDC__
-#define unlink(s) remove(s)
-#endif
-
  #ifdef HAVE_UNISTD_H
  #include <unistd.h>		/* For SEEK_SET etc.  */
  #endif
@@ -303,18 +300,19 @@
  SWIopen (ARMul_State * state, ARMword name, ARMword SWIflags)
 {
   struct OSblock *OSptr = (struct OSblock *) state->OSptr;
-  char dummy[2000];
+  char buf[PATH_MAX], *p = buf;
   int flags;
   int i;
 
-  for (i = 0; (dummy[i] = ARMul_SafeReadByte (state, name + i)); i++)
-    ;
+  for (i = 0; i < sizeof buf; i++)
+    if ((*p++ = ARMul_SafeReadByte (state, name++)) == '\0')
+      break;
 
   /* Now we need to decode the Demon open mode.  */
   flags = translate_open_mode[SWIflags];
 
   /* Filename ":tt" is special: it denotes stdin/out.  */
-  if (strcmp (dummy, ":tt") == 0)
+  if (strcmp (buf, ":tt") == 0)
     {
       if (flags == TARGET_O_RDONLY) /* opening tty "r" */
  	state->Reg[0] = 0;	/* stdin */
@@ -323,7 +321,7 @@
     }
   else
     {
-      state->Reg[0] = sim_callback->open (sim_callback, dummy, flags);
+      state->Reg[0] = sim_callback->open (sim_callback, buf, flags);
       OSptr->ErrorNo = sim_callback->get_errno (sim_callback);
     }
 }
@@ -403,6 +401,39 @@
   OSptr->ErrorNo = sim_callback->get_errno (sim_callback);
 }
 
+static void
+SWIremove (ARMul_State * state, ARMword path)
+{
+  struct OSblock *OSptr = (struct OSblock *) state->OSptr;
+  char buf[PATH_MAX], *p = buf;
+  int i;
+
+  for (i = 0; i < sizeof buf; i++)
+    if ((*p++ = ARMul_SafeReadByte (state, path++)) == '\0')
+      break;
+
+  state->Reg[0] = sim_callback->unlink (sim_callback, buf);
+  OSptr->ErrorNo = sim_callback->get_errno (sim_callback);
+}
+
+static void
+SWIrename (ARMul_State * state, ARMword old, ARMword new)
+{
+  struct OSblock *OSptr = (struct OSblock *) state->OSptr;
+  char oldbuf[PATH_MAX], newbuf[PATH_MAX], *p;
+  int i;
+
+  for (p = oldbuf, i = 0; i < sizeof oldbuf; i++)
+    if ((*p++ = ARMul_SafeReadByte (state, old++)) == '\0')
+      break;
+  for (p = newbuf, i = 0; i < sizeof newbuf; i++)
+    if ((*p++ = ARMul_SafeReadByte (state, new++)) == '\0')
+      break;
+
+  state->Reg[0] = sim_callback->rename (sim_callback, oldbuf, newbuf);
+  OSptr->ErrorNo = sim_callback->get_errno (sim_callback);
+}
+
  /* The emulator calls this routine when a SWI instruction is encuntered.
    The parameter passed is the SWI number (lower 24 bits of the
instruction).  */
 
@@ -544,6 +575,30 @@
       state->Emulate = FALSE;
       break;
 
+    case SWI_Remove:
+      if (swi_mask & SWI_MASK_DEMON)
+	SWIremove (state, state->Reg[0]);
+      else
+	unhandled = TRUE;
+      break;
+
+    case SWI_Rename:
+      if (swi_mask & SWI_MASK_DEMON)
+	SWIrename (state, state->Reg[0], state->Reg[1]);
+      else
+	unhandled = TRUE;
+      break;
+
+    case SWI_IsTTY:
+      if (swi_mask & SWI_MASK_DEMON)
+	{
+	  state->Reg[0] = sim_callback->isatty (sim_callback, state->Reg[0]);
+	  OSptr->ErrorNo = sim_callback->get_errno (sim_callback);
+	}
+      else
+	unhandled = TRUE;
+      break;
+
       /* Handle Angel SWIs as well as Demon ones.  */
     case AngelSWI_ARM:
     case AngelSWI_Thumb:
@@ -566,10 +621,7 @@
 	  
  	      /* Unimplemented reason codes.  */
 	    case AngelSWI_Reason_ReadC:
-	    case AngelSWI_Reason_IsTTY:
 	    case AngelSWI_Reason_TmpNam:
-	    case AngelSWI_Reason_Remove:
-	    case AngelSWI_Reason_Rename:
 	    case AngelSWI_Reason_System:
 	    case AngelSWI_Reason_EnterSVC:
  	    default:
@@ -684,6 +736,21 @@
  			ARMul_ReadWord (state, addr + 4),
  			ARMul_ReadWord (state, addr + 8));
 	      break;
+
+	    case AngelSWI_Reason_IsTTY:
+	      state->Reg[0] = sim_callback->close (sim_callback,
+						   ARMul_ReadWord (state, addr));
+	      OSptr->ErrorNo = sim_callback->get_errno (sim_callback);
+	      break;
+
+	    case AngelSWI_Reason_Remove:
+	      SWIremove (state,
+			 ARMul_ReadWord (state, addr));
+
+	    case AngelSWI_Reason_Rename:
+	      SWIrename (state,
+			 ARMul_ReadWord (state, addr),
+			 ARMul_ReadWord (state, addr + 4));
 	    }
 	}
       else

[-- Attachment #2: sim-arm-swi.diff --]
[-- Type: text/plain, Size: 4922 bytes --]

2005-09-07  Shaun Jackman  <sjackman@gmail.com>

	* sim/arm/armos.c (unlink): Remove this macro. It is unused
	in this file and conflicts with sim_callback->unlink.
	(SWIopen): Fix a potential buffer overflow.
	(SWIremove): New function.
	(SWIrename): Ditto.
	(ARMul_OSHandleSWI): Handle the RDP calls SWI_IsTTY,
	SWI_Remove, and SWI_Rename, as well as the RDI calls
	AngelSWI_Reason_IsTTY, AngelSWI_Reason_Remove, and
	AngelSWI_Reason_Rename.

Index: sim/arm/armos.c
===================================================================
RCS file: /cvs/src/src/sim/arm/armos.c,v
retrieving revision 1.22
diff -u -r1.22 armos.c
--- sim/arm/armos.c	12 May 2005 07:36:58 -0000	1.22
+++ sim/arm/armos.c	7 Sep 2005 16:45:27 -0000
@@ -27,6 +27,7 @@
 
 #include <time.h>
 #include <errno.h>
+#include <limits.h>
 #include <string.h>
 #include "targ-vals.h"
 
@@ -34,10 +35,6 @@
 #define TARGET_O_BINARY 0
 #endif
 
-#ifdef __STDC__
-#define unlink(s) remove(s)
-#endif
-
 #ifdef HAVE_UNISTD_H
 #include <unistd.h>		/* For SEEK_SET etc.  */
 #endif
@@ -303,18 +300,19 @@
 SWIopen (ARMul_State * state, ARMword name, ARMword SWIflags)
 {
   struct OSblock *OSptr = (struct OSblock *) state->OSptr;
-  char dummy[2000];
+  char buf[PATH_MAX], *p = buf;
   int flags;
   int i;
 
-  for (i = 0; (dummy[i] = ARMul_SafeReadByte (state, name + i)); i++)
-    ;
+  for (i = 0; i < sizeof buf; i++)
+    if ((*p++ = ARMul_SafeReadByte (state, name++)) == '\0')
+      break;
 
   /* Now we need to decode the Demon open mode.  */
   flags = translate_open_mode[SWIflags];
 
   /* Filename ":tt" is special: it denotes stdin/out.  */
-  if (strcmp (dummy, ":tt") == 0)
+  if (strcmp (buf, ":tt") == 0)
     {
       if (flags == TARGET_O_RDONLY) /* opening tty "r" */
 	state->Reg[0] = 0;	/* stdin */
@@ -323,7 +321,7 @@
     }
   else
     {
-      state->Reg[0] = sim_callback->open (sim_callback, dummy, flags);
+      state->Reg[0] = sim_callback->open (sim_callback, buf, flags);
       OSptr->ErrorNo = sim_callback->get_errno (sim_callback);
     }
 }
@@ -403,6 +401,39 @@
   OSptr->ErrorNo = sim_callback->get_errno (sim_callback);
 }
 
+static void
+SWIremove (ARMul_State * state, ARMword path)
+{
+  struct OSblock *OSptr = (struct OSblock *) state->OSptr;
+  char buf[PATH_MAX], *p = buf;
+  int i;
+
+  for (i = 0; i < sizeof buf; i++)
+    if ((*p++ = ARMul_SafeReadByte (state, path++)) == '\0')
+      break;
+
+  state->Reg[0] = sim_callback->unlink (sim_callback, buf);
+  OSptr->ErrorNo = sim_callback->get_errno (sim_callback);
+}
+
+static void
+SWIrename (ARMul_State * state, ARMword old, ARMword new)
+{
+  struct OSblock *OSptr = (struct OSblock *) state->OSptr;
+  char oldbuf[PATH_MAX], newbuf[PATH_MAX], *p;
+  int i;
+
+  for (p = oldbuf, i = 0; i < sizeof oldbuf; i++)
+    if ((*p++ = ARMul_SafeReadByte (state, old++)) == '\0')
+      break;
+  for (p = newbuf, i = 0; i < sizeof newbuf; i++)
+    if ((*p++ = ARMul_SafeReadByte (state, new++)) == '\0')
+      break;
+
+  state->Reg[0] = sim_callback->rename (sim_callback, oldbuf, newbuf);
+  OSptr->ErrorNo = sim_callback->get_errno (sim_callback);
+}
+
 /* The emulator calls this routine when a SWI instruction is encuntered.
    The parameter passed is the SWI number (lower 24 bits of the instruction).  */
 
@@ -544,6 +575,30 @@
       state->Emulate = FALSE;
       break;
 
+    case SWI_Remove:
+      if (swi_mask & SWI_MASK_DEMON)
+	SWIremove (state, state->Reg[0]);
+      else
+	unhandled = TRUE;
+      break;
+
+    case SWI_Rename:
+      if (swi_mask & SWI_MASK_DEMON)
+	SWIrename (state, state->Reg[0], state->Reg[1]);
+      else
+	unhandled = TRUE;
+      break;
+
+    case SWI_IsTTY:
+      if (swi_mask & SWI_MASK_DEMON)
+	{
+	  state->Reg[0] = sim_callback->isatty (sim_callback, state->Reg[0]);
+	  OSptr->ErrorNo = sim_callback->get_errno (sim_callback);
+	}
+      else
+	unhandled = TRUE;
+      break;
+
       /* Handle Angel SWIs as well as Demon ones.  */
     case AngelSWI_ARM:
     case AngelSWI_Thumb:
@@ -566,10 +621,7 @@
 	  
 	      /* Unimplemented reason codes.  */
 	    case AngelSWI_Reason_ReadC:
-	    case AngelSWI_Reason_IsTTY:
 	    case AngelSWI_Reason_TmpNam:
-	    case AngelSWI_Reason_Remove:
-	    case AngelSWI_Reason_Rename:
 	    case AngelSWI_Reason_System:
 	    case AngelSWI_Reason_EnterSVC:
 	    default:
@@ -684,6 +736,21 @@
 			ARMul_ReadWord (state, addr + 4),
 			ARMul_ReadWord (state, addr + 8));
 	      break;
+
+	    case AngelSWI_Reason_IsTTY:
+	      state->Reg[0] = sim_callback->close (sim_callback,
+						   ARMul_ReadWord (state, addr));
+	      OSptr->ErrorNo = sim_callback->get_errno (sim_callback);
+	      break;
+
+	    case AngelSWI_Reason_Remove:
+	      SWIremove (state,
+			 ARMul_ReadWord (state, addr));
+
+	    case AngelSWI_Reason_Rename:
+	      SWIrename (state,
+			 ARMul_ReadWord (state, addr),
+			 ARMul_ReadWord (state, addr + 4));
 	    }
 	}
       else

Re: sim/arm/armos.c: IsTTY [PATCH]

[Richard Earnshaw]

On Wed, 2005-09-07 at 17:51, Shaun Jackman wrote:
> 2005/8/29, Daniel Jacobowitz <drow@false.org>:
> > Please no buffer overflows be adding.  Also, there's plenty of ways to
> > write this without the ugly empty loop body...
> 
> I fixed the potential buffer overflow and cleaned up the loop coding
> style. Thanks for the tips.
> 
> Cheers,
> Shaun
> 
> 2005-09-07  Shaun Jackman  <sjackman@gmail.com>
> 
> 	* sim/arm/armos.c (unlink): Remove this macro. It is unused
> 	in this file and conflicts with sim_callback->unlink.
> 	(SWIopen): Fix a potential buffer overflow.
> 	(SWIremove): New function.
> 	(SWIrename): Ditto.
> 	(ARMul_OSHandleSWI): Handle the RDP calls SWI_IsTTY,
> 	SWI_Remove, and SWI_Rename, as well as the RDI calls
> 	AngelSWI_Reason_IsTTY, AngelSWI_Reason_Remove, and
> 	AngelSWI_Reason_Rename.
> 

Two problems I can immediately see with this.

1) PATH_MAX isn't ANSI (it's POSIX, or something like that).  So you
can't rely on it being defined.  I think for this case you can probably
just define it to 1024 anyway if it's missing, but see
libiberty/lrealpath.c if you want the gory details.

2) If you do overflow the path limit, you need to set the simulator's
errno value and return.  Use cb_host_to_target_errno(sim_callback,
ENAMETOOLONG) to set it.

R.

Re: sim/arm/armos.c: IsTTY [PATCH]

[Eli Zaretskii]

> From: Richard Earnshaw <rearnsha@gcc.gnu.org>
> Cc: gdb-patches@sources.redhat.com
> Date: Thu, 08 Sep 2005 10:06:28 +0100
> 
> 1) PATH_MAX isn't ANSI (it's POSIX, or something like that).  So you
> can't rely on it being defined.  I think for this case you can probably
> just define it to 1024 anyway if it's missing, but see
> libiberty/lrealpath.c if you want the gory details.

Or perhaps just use FILENAME_MAX, which _is_ ANSI.

Re: sim/arm/armos.c: IsTTY [PATCH]

[Shaun Jackman]

2005/9/8, Richard Earnshaw <rearnsha@gcc.gnu.org>:
> 1) PATH_MAX isn't ANSI (it's POSIX, or something like that).  So you
> can't rely on it being defined.  I think for this case you can probably
> just define it to 1024 anyway if it's missing, but see
> libiberty/lrealpath.c if you want the gory details.

I'll use the ANSI FILENAME_MAX as Eli suggested.

> 2) If you do overflow the path limit, you need to set the simulator's
> errno value and return.  Use cb_host_to_target_errno(sim_callback,
> ENAMETOOLONG) to set it.

Is this the correct way to use cb_host_to_target_errno?

  for (i = 0; i < sizeof buf; i++)
    if ((*p++ = ARMul_SafeReadByte (state, name++)) == '\0')
      break;
  if (i == sizeof buf) {
    state->Reg[0] = -1;
    OSptr->ErrorNo = cb_host_to_target_errno(sim_callback, ENAMETOOLONG);
    return;
  }

Cheers,
Shaun

Re: sim/arm/armos.c: IsTTY [PATCH]

[Shaun Jackman]

[-- Attachment #1: Type: text/plain, Size: 6391 bytes --]

2005/9/8, Richard Earnshaw <rearnsha@gcc.gnu.org>:
> Two problems I can immediately see with this.
> 
> 1) PATH_MAX isn't ANSI (it's POSIX, or something like that).  So you
> can't rely on it being defined.  I think for this case you can probably
> just define it to 1024 anyway if it's missing, but see
> libiberty/lrealpath.c if you want the gory details.
> 
> 2) If you do overflow the path limit, you need to set the simulator's
> errno value and return.  Use cb_host_to_target_errno(sim_callback,
> ENAMETOOLONG) to set it.
> 
> R.

I've changed PATH_MAX from POSIX to FILENAME_MAX from ANSI and checked
if the path limit overflowed.

Cheers,
Shaun

2005-09-09  Shaun Jackman  <sjackman@gmail.com>

	* sim/arm/armos.c (unlink): Remove this macro. It is unused
	in this file and conflicts with sim_callback->unlink.
	(SWIopen): Fix a potential buffer overflow.
	(SWIremove): New function.
	(SWIrename): Ditto.
	(ARMul_OSHandleSWI): Handle the RDP calls SWI_IsTTY,
	SWI_Remove, and SWI_Rename, as well as the RDI calls
	AngelSWI_Reason_IsTTY, AngelSWI_Reason_Remove, and
	AngelSWI_Reason_Rename.

Index: sim/arm/armos.c
===================================================================
RCS file: /cvs/src/src/sim/arm/armos.c,v
retrieving revision 1.22
diff -u -r1.22 armos.c
--- sim/arm/armos.c	12 May 2005 07:36:58 -0000	1.22
+++ sim/arm/armos.c	9 Sep 2005 17:19:08 -0000
@@ -27,6 +27,7 @@
 
 #include <time.h>
 #include <errno.h>
+#include <limits.h>
 #include <string.h>
 #include "targ-vals.h"
 
@@ -34,10 +35,6 @@
 #define TARGET_O_BINARY 0
 #endif
 
-#ifdef __STDC__
-#define unlink(s) remove(s)
-#endif
-
 #ifdef HAVE_UNISTD_H
 #include <unistd.h>		/* For SEEK_SET etc.  */
 #endif
@@ -303,18 +300,24 @@
 SWIopen (ARMul_State * state, ARMword name, ARMword SWIflags)
 {
   struct OSblock *OSptr = (struct OSblock *) state->OSptr;
-  char dummy[2000];
+  char buf[FILENAME_MAX], *p = buf;
   int flags;
   int i;
 
-  for (i = 0; (dummy[i] = ARMul_SafeReadByte (state, name + i)); i++)
-    ;
+  for (i = 0; i < sizeof buf; i++)
+    if ((*p++ = ARMul_SafeReadByte (state, name++)) == '\0')
+      break;
+  if (i == sizeof buf) {
+    state->Reg[0] = -1;
+    OSptr->ErrorNo = cb_host_to_target_errno(sim_callback, ENAMETOOLONG);
+    return;
+  }
 
   /* Now we need to decode the Demon open mode.  */
   flags = translate_open_mode[SWIflags];
 
   /* Filename ":tt" is special: it denotes stdin/out.  */
-  if (strcmp (dummy, ":tt") == 0)
+  if (strcmp (buf, ":tt") == 0)
     {
       if (flags == TARGET_O_RDONLY) /* opening tty "r" */
 	state->Reg[0] = 0;	/* stdin */
@@ -323,7 +326,7 @@
     }
   else
     {
-      state->Reg[0] = sim_callback->open (sim_callback, dummy, flags);
+      state->Reg[0] = sim_callback->open (sim_callback, buf, flags);
       OSptr->ErrorNo = sim_callback->get_errno (sim_callback);
     }
 }
@@ -403,6 +406,54 @@
   OSptr->ErrorNo = sim_callback->get_errno (sim_callback);
 }
 
+static void
+SWIremove (ARMul_State * state, ARMword path)
+{
+  struct OSblock *OSptr = (struct OSblock *) state->OSptr;
+  char buf[FILENAME_MAX], *p = buf;
+  int i;
+
+  for (i = 0; i < sizeof buf; i++)
+    if ((*p++ = ARMul_SafeReadByte (state, path++)) == '\0')
+      break;
+  if (i == sizeof buf) {
+    state->Reg[0] = -1;
+    OSptr->ErrorNo = cb_host_to_target_errno(sim_callback, ENAMETOOLONG);
+    return;
+  }
+
+  state->Reg[0] = sim_callback->unlink (sim_callback, buf);
+  OSptr->ErrorNo = sim_callback->get_errno (sim_callback);
+}
+
+static void
+SWIrename (ARMul_State * state, ARMword old, ARMword new)
+{
+  struct OSblock *OSptr = (struct OSblock *) state->OSptr;
+  char oldbuf[FILENAME_MAX], newbuf[FILENAME_MAX], *p;
+  int i;
+
+  for (p = oldbuf, i = 0; i < sizeof oldbuf; i++)
+    if ((*p++ = ARMul_SafeReadByte (state, old++)) == '\0')
+      break;
+  if (i == sizeof oldbuf) {
+    state->Reg[0] = -1;
+    OSptr->ErrorNo = cb_host_to_target_errno(sim_callback, ENAMETOOLONG);
+    return;
+  }
+  for (p = newbuf, i = 0; i < sizeof newbuf; i++)
+    if ((*p++ = ARMul_SafeReadByte (state, new++)) == '\0')
+      break;
+  if (i == sizeof newbuf) {
+    state->Reg[0] = -1;
+    OSptr->ErrorNo = cb_host_to_target_errno(sim_callback, ENAMETOOLONG);
+    return;
+  }
+
+  state->Reg[0] = sim_callback->rename (sim_callback, oldbuf, newbuf);
+  OSptr->ErrorNo = sim_callback->get_errno (sim_callback);
+}
+
 /* The emulator calls this routine when a SWI instruction is encuntered.
    The parameter passed is the SWI number (lower 24 bits of the
instruction).  */
 
@@ -544,6 +595,30 @@
       state->Emulate = FALSE;
       break;
 
+    case SWI_Remove:
+      if (swi_mask & SWI_MASK_DEMON)
+	SWIremove (state, state->Reg[0]);
+      else
+	unhandled = TRUE;
+      break;
+
+    case SWI_Rename:
+      if (swi_mask & SWI_MASK_DEMON)
+	SWIrename (state, state->Reg[0], state->Reg[1]);
+      else
+	unhandled = TRUE;
+      break;
+
+    case SWI_IsTTY:
+      if (swi_mask & SWI_MASK_DEMON)
+	{
+	  state->Reg[0] = sim_callback->isatty (sim_callback, state->Reg[0]);
+	  OSptr->ErrorNo = sim_callback->get_errno (sim_callback);
+	}
+      else
+	unhandled = TRUE;
+      break;
+
       /* Handle Angel SWIs as well as Demon ones.  */
     case AngelSWI_ARM:
     case AngelSWI_Thumb:
@@ -566,10 +641,7 @@
 	  
 	      /* Unimplemented reason codes.  */
 	    case AngelSWI_Reason_ReadC:
-	    case AngelSWI_Reason_IsTTY:
 	    case AngelSWI_Reason_TmpNam:
-	    case AngelSWI_Reason_Remove:
-	    case AngelSWI_Reason_Rename:
 	    case AngelSWI_Reason_System:
 	    case AngelSWI_Reason_EnterSVC:
 	    default:
@@ -684,6 +756,21 @@
 			ARMul_ReadWord (state, addr + 4),
 			ARMul_ReadWord (state, addr + 8));
 	      break;
+
+	    case AngelSWI_Reason_IsTTY:
+	      state->Reg[0] = sim_callback->close (sim_callback,
+						   ARMul_ReadWord (state, addr));
+	      OSptr->ErrorNo = sim_callback->get_errno (sim_callback);
+	      break;
+
+	    case AngelSWI_Reason_Remove:
+	      SWIremove (state,
+			 ARMul_ReadWord (state, addr));
+
+	    case AngelSWI_Reason_Rename:
+	      SWIrename (state,
+			 ARMul_ReadWord (state, addr),
+			 ARMul_ReadWord (state, addr + 4));
 	    }
 	}
       else

[-- Attachment #2: sim-arm-swi.diff --]
[-- Type: text/plain, Size: 5520 bytes --]

2005-09-09  Shaun Jackman  <sjackman@gmail.com>

	* sim/arm/armos.c (unlink): Remove this macro. It is unused
	in this file and conflicts with sim_callback->unlink.
	(SWIopen): Fix a potential buffer overflow.
	(SWIremove): New function.
	(SWIrename): Ditto.
	(ARMul_OSHandleSWI): Handle the RDP calls SWI_IsTTY,
	SWI_Remove, and SWI_Rename, as well as the RDI calls
	AngelSWI_Reason_IsTTY, AngelSWI_Reason_Remove, and
	AngelSWI_Reason_Rename.

Index: sim/arm/armos.c
===================================================================
RCS file: /cvs/src/src/sim/arm/armos.c,v
retrieving revision 1.22
diff -u -r1.22 armos.c
--- sim/arm/armos.c	12 May 2005 07:36:58 -0000	1.22
+++ sim/arm/armos.c	9 Sep 2005 17:19:08 -0000
@@ -27,6 +27,7 @@
 
 #include <time.h>
 #include <errno.h>
+#include <limits.h>
 #include <string.h>
 #include "targ-vals.h"
 
@@ -34,10 +35,6 @@
 #define TARGET_O_BINARY 0
 #endif
 
-#ifdef __STDC__
-#define unlink(s) remove(s)
-#endif
-
 #ifdef HAVE_UNISTD_H
 #include <unistd.h>		/* For SEEK_SET etc.  */
 #endif
@@ -303,18 +300,24 @@
 SWIopen (ARMul_State * state, ARMword name, ARMword SWIflags)
 {
   struct OSblock *OSptr = (struct OSblock *) state->OSptr;
-  char dummy[2000];
+  char buf[FILENAME_MAX], *p = buf;
   int flags;
   int i;
 
-  for (i = 0; (dummy[i] = ARMul_SafeReadByte (state, name + i)); i++)
-    ;
+  for (i = 0; i < sizeof buf; i++)
+    if ((*p++ = ARMul_SafeReadByte (state, name++)) == '\0')
+      break;
+  if (i == sizeof buf) {
+    state->Reg[0] = -1;
+    OSptr->ErrorNo = cb_host_to_target_errno(sim_callback, ENAMETOOLONG);
+    return;
+  }
 
   /* Now we need to decode the Demon open mode.  */
   flags = translate_open_mode[SWIflags];
 
   /* Filename ":tt" is special: it denotes stdin/out.  */
-  if (strcmp (dummy, ":tt") == 0)
+  if (strcmp (buf, ":tt") == 0)
     {
       if (flags == TARGET_O_RDONLY) /* opening tty "r" */
 	state->Reg[0] = 0;	/* stdin */
@@ -323,7 +326,7 @@
     }
   else
     {
-      state->Reg[0] = sim_callback->open (sim_callback, dummy, flags);
+      state->Reg[0] = sim_callback->open (sim_callback, buf, flags);
       OSptr->ErrorNo = sim_callback->get_errno (sim_callback);
     }
 }
@@ -403,6 +406,54 @@
   OSptr->ErrorNo = sim_callback->get_errno (sim_callback);
 }
 
+static void
+SWIremove (ARMul_State * state, ARMword path)
+{
+  struct OSblock *OSptr = (struct OSblock *) state->OSptr;
+  char buf[FILENAME_MAX], *p = buf;
+  int i;
+
+  for (i = 0; i < sizeof buf; i++)
+    if ((*p++ = ARMul_SafeReadByte (state, path++)) == '\0')
+      break;
+  if (i == sizeof buf) {
+    state->Reg[0] = -1;
+    OSptr->ErrorNo = cb_host_to_target_errno(sim_callback, ENAMETOOLONG);
+    return;
+  }
+
+  state->Reg[0] = sim_callback->unlink (sim_callback, buf);
+  OSptr->ErrorNo = sim_callback->get_errno (sim_callback);
+}
+
+static void
+SWIrename (ARMul_State * state, ARMword old, ARMword new)
+{
+  struct OSblock *OSptr = (struct OSblock *) state->OSptr;
+  char oldbuf[FILENAME_MAX], newbuf[FILENAME_MAX], *p;
+  int i;
+
+  for (p = oldbuf, i = 0; i < sizeof oldbuf; i++)
+    if ((*p++ = ARMul_SafeReadByte (state, old++)) == '\0')
+      break;
+  if (i == sizeof oldbuf) {
+    state->Reg[0] = -1;
+    OSptr->ErrorNo = cb_host_to_target_errno(sim_callback, ENAMETOOLONG);
+    return;
+  }
+  for (p = newbuf, i = 0; i < sizeof newbuf; i++)
+    if ((*p++ = ARMul_SafeReadByte (state, new++)) == '\0')
+      break;
+  if (i == sizeof newbuf) {
+    state->Reg[0] = -1;
+    OSptr->ErrorNo = cb_host_to_target_errno(sim_callback, ENAMETOOLONG);
+    return;
+  }
+
+  state->Reg[0] = sim_callback->rename (sim_callback, oldbuf, newbuf);
+  OSptr->ErrorNo = sim_callback->get_errno (sim_callback);
+}
+
 /* The emulator calls this routine when a SWI instruction is encuntered.
    The parameter passed is the SWI number (lower 24 bits of the instruction).  */
 
@@ -544,6 +595,30 @@
       state->Emulate = FALSE;
       break;
 
+    case SWI_Remove:
+      if (swi_mask & SWI_MASK_DEMON)
+	SWIremove (state, state->Reg[0]);
+      else
+	unhandled = TRUE;
+      break;
+
+    case SWI_Rename:
+      if (swi_mask & SWI_MASK_DEMON)
+	SWIrename (state, state->Reg[0], state->Reg[1]);
+      else
+	unhandled = TRUE;
+      break;
+
+    case SWI_IsTTY:
+      if (swi_mask & SWI_MASK_DEMON)
+	{
+	  state->Reg[0] = sim_callback->isatty (sim_callback, state->Reg[0]);
+	  OSptr->ErrorNo = sim_callback->get_errno (sim_callback);
+	}
+      else
+	unhandled = TRUE;
+      break;
+
       /* Handle Angel SWIs as well as Demon ones.  */
     case AngelSWI_ARM:
     case AngelSWI_Thumb:
@@ -566,10 +641,7 @@
 	  
 	      /* Unimplemented reason codes.  */
 	    case AngelSWI_Reason_ReadC:
-	    case AngelSWI_Reason_IsTTY:
 	    case AngelSWI_Reason_TmpNam:
-	    case AngelSWI_Reason_Remove:
-	    case AngelSWI_Reason_Rename:
 	    case AngelSWI_Reason_System:
 	    case AngelSWI_Reason_EnterSVC:
 	    default:
@@ -684,6 +756,21 @@
 			ARMul_ReadWord (state, addr + 4),
 			ARMul_ReadWord (state, addr + 8));
 	      break;
+
+	    case AngelSWI_Reason_IsTTY:
+	      state->Reg[0] = sim_callback->close (sim_callback,
+						   ARMul_ReadWord (state, addr));
+	      OSptr->ErrorNo = sim_callback->get_errno (sim_callback);
+	      break;
+
+	    case AngelSWI_Reason_Remove:
+	      SWIremove (state,
+			 ARMul_ReadWord (state, addr));
+
+	    case AngelSWI_Reason_Rename:
+	      SWIrename (state,
+			 ARMul_ReadWord (state, addr),
+			 ARMul_ReadWord (state, addr + 4));
 	    }
 	}
       else

Re: sim/arm/armos.c: IsTTY [PATCH]

[Daniel Jacobowitz]

On Fri, Sep 09, 2005 at 11:23:14AM -0600, Shaun Jackman wrote:
> 2005/9/8, Richard Earnshaw <rearnsha@gcc.gnu.org>:
> > Two problems I can immediately see with this.
> > 
> > 1) PATH_MAX isn't ANSI (it's POSIX, or something like that).  So you
> > can't rely on it being defined.  I think for this case you can probably
> > just define it to 1024 anyway if it's missing, but see
> > libiberty/lrealpath.c if you want the gory details.
> > 
> > 2) If you do overflow the path limit, you need to set the simulator's
> > errno value and return.  Use cb_host_to_target_errno(sim_callback,
> > ENAMETOOLONG) to set it.
> > 
> > R.
> 
> I've changed PATH_MAX from POSIX to FILENAME_MAX from ANSI and checked
> if the path limit overflowed.

I'm sorry we even got into this... but no, you can not use FILENAME_MAX
this way:

Macro: int FILENAME_MAX
    The value of this macro is an integer constant expression that
represents the maximum length of a file name string. It is defined in
`stdio.h'.

    Unlike PATH_MAX, this macro is defined even if there is no actual
limit imposed. In such a case, its value is typically a very large
number. This is always the case on the GNU system.

    Usage Note: Don't use FILENAME_MAX as the size of an array in which
to store a file name! You can't possibly make an array that big! Use
dynamic allocation (see section 3.2 Allocating Storage For Program
Data) instead. 


That's from the glibc manual, but it's generally good advice.  I don't
know if Hurd does this at the moment, looking at current glibc, but it
certainly did at one point.  Also, some older systems defined
FILENAME_MAX as 14.

I'd recommend following Richard's suggestion: use PATH_MAX, if it's not
there, default to 1K.

> -  for (i = 0; (dummy[i] = ARMul_SafeReadByte (state, name + i)); i++)
> -    ;
> +  for (i = 0; i < sizeof buf; i++)
> +    if ((*p++ = ARMul_SafeReadByte (state, name++)) == '\0')
> +      break;
> +  if (i == sizeof buf) {
> +    state->Reg[0] = -1;
> +    OSptr->ErrorNo = cb_host_to_target_errno(sim_callback, ENAMETOOLONG);
> +    return;
> +  }

You might want a helper function for this, since it happens in a lot of
places.  Also, braces on their own lines.

Otherwise it looks fine to me.


-- 
Daniel Jacobowitz
CodeSourcery, LLC

Re: sim/arm/armos.c: IsTTY [PATCH]

[Shaun Jackman]

[-- Attachment #1: Type: text/plain, Size: 5900 bytes --]

2005/9/17, Daniel Jacobowitz <drow@false.org>:
> I'd recommend following Richard's suggestion: use PATH_MAX, if it's not
> there, default to 1K.

OK.

> You might want a helper function for this, since it happens in a lot of
> places.  Also, braces on their own lines.

Done.
 
> Otherwise it looks fine to me.

Great!

Cheers,
Shaun

2005-09-17  Shaun Jackman  <sjackman@gmail.com>

	* sim/arm/armos.c: Include limits.h
	(unlink): Remove this macro. It is unused in this file and
	conflicts with sim_callback->unlink.
	(PATH_MAX): Define as 1024 if not already defined.
	(ReadFileName): New function.
	(SWIopen): Fix a potential buffer overflow.
	(SWIremove): New function.
	(SWIrename): Ditto.
	(ARMul_OSHandleSWI): Handle the RDP calls SWI_IsTTY,
	SWI_Remove, and SWI_Rename, as well as the RDI calls
	AngelSWI_Reason_IsTTY, AngelSWI_Reason_Remove, and
	AngelSWI_Reason_Rename.

Index: sim/arm/armos.c
===================================================================
RCS file: /cvs/src/src/sim/arm/armos.c,v
retrieving revision 1.22
diff -u -r1.22 armos.c
--- sim/arm/armos.c	12 May 2005 07:36:58 -0000	1.22
+++ sim/arm/armos.c	17 Sep 2005 23:24:51 -0000
@@ -27,6 +27,7 @@
 
 #include <time.h>
 #include <errno.h>
+#include <limits.h>
 #include <string.h>
 #include "targ-vals.h"
 
@@ -34,10 +35,6 @@
 #define TARGET_O_BINARY 0
 #endif
 
-#ifdef __STDC__
-#define unlink(s) remove(s)
-#endif
-
 #ifdef HAVE_UNISTD_H
 #include <unistd.h>		/* For SEEK_SET etc.  */
 #endif
@@ -89,6 +86,9 @@
 #define FOPEN_MAX 64
 #endif
 #define UNIQUETEMPS 256
+#ifndef PATH_MAX
+#define PATH_MAX 1024
+#endif
 
 /* OS private Information.  */
 
@@ -299,22 +299,34 @@
   while (temp != 0);
 }
 
+static int
+ReadFileName(ARMul_State *state, char *buf, ARMword src, size_t n)
+{
+  struct OSblock *OSptr = (struct OSblock *) state->OSptr;
+  char *p = buf;
+
+  while (n--)
+    if ((*p++ = ARMul_SafeReadByte (state, src++)) == '\0')
+      return 0;
+  OSptr->ErrorNo = cb_host_to_target_errno(sim_callback, ENAMETOOLONG);
+  return state->Reg[0] = -1;
+}
+
 static void
 SWIopen (ARMul_State * state, ARMword name, ARMword SWIflags)
 {
   struct OSblock *OSptr = (struct OSblock *) state->OSptr;
-  char dummy[2000];
+  char buf[PATH_MAX];
   int flags;
-  int i;
 
-  for (i = 0; (dummy[i] = ARMul_SafeReadByte (state, name + i)); i++)
-    ;
+  if (ReadFileName(state, buf, name, sizeof buf) == -1)
+    return;
 
   /* Now we need to decode the Demon open mode.  */
   flags = translate_open_mode[SWIflags];
 
   /* Filename ":tt" is special: it denotes stdin/out.  */
-  if (strcmp (dummy, ":tt") == 0)
+  if (strcmp (buf, ":tt") == 0)
     {
       if (flags == TARGET_O_RDONLY) /* opening tty "r" */
 	state->Reg[0] = 0;	/* stdin */
@@ -323,7 +335,7 @@
     }
   else
     {
-      state->Reg[0] = sim_callback->open (sim_callback, dummy, flags);
+      state->Reg[0] = sim_callback->open (sim_callback, buf, flags);
       OSptr->ErrorNo = sim_callback->get_errno (sim_callback);
     }
 }
@@ -403,6 +415,33 @@
   OSptr->ErrorNo = sim_callback->get_errno (sim_callback);
 }
 
+static void
+SWIremove (ARMul_State * state, ARMword path)
+{
+  char buf[PATH_MAX];
+
+  if (ReadFileName(state, buf, path, sizeof buf) != -1)
+  {
+    struct OSblock *OSptr = (struct OSblock *) state->OSptr;
+    state->Reg[0] = sim_callback->unlink (sim_callback, buf);
+    OSptr->ErrorNo = sim_callback->get_errno (sim_callback);
+  }
+}
+
+static void
+SWIrename (ARMul_State * state, ARMword old, ARMword new)
+{
+  char oldbuf[PATH_MAX], newbuf[PATH_MAX];
+
+  if (ReadFileName(state, oldbuf, old, sizeof oldbuf) != -1 &&
+      ReadFileName(state, newbuf, new, sizeof newbuf) != -1)
+  {
+    struct OSblock *OSptr = (struct OSblock *) state->OSptr;
+    state->Reg[0] = sim_callback->rename (sim_callback, oldbuf, newbuf);
+    OSptr->ErrorNo = sim_callback->get_errno (sim_callback);
+  }
+}
+
 /* The emulator calls this routine when a SWI instruction is encuntered.
    The parameter passed is the SWI number (lower 24 bits of the
instruction).  */
 
@@ -544,6 +583,30 @@
       state->Emulate = FALSE;
       break;
 
+    case SWI_Remove:
+      if (swi_mask & SWI_MASK_DEMON)
+	SWIremove (state, state->Reg[0]);
+      else
+	unhandled = TRUE;
+      break;
+
+    case SWI_Rename:
+      if (swi_mask & SWI_MASK_DEMON)
+	SWIrename (state, state->Reg[0], state->Reg[1]);
+      else
+	unhandled = TRUE;
+      break;
+
+    case SWI_IsTTY:
+      if (swi_mask & SWI_MASK_DEMON)
+	{
+	  state->Reg[0] = sim_callback->isatty (sim_callback, state->Reg[0]);
+	  OSptr->ErrorNo = sim_callback->get_errno (sim_callback);
+	}
+      else
+	unhandled = TRUE;
+      break;
+
       /* Handle Angel SWIs as well as Demon ones.  */
     case AngelSWI_ARM:
     case AngelSWI_Thumb:
@@ -566,10 +629,7 @@
 	  
 	      /* Unimplemented reason codes.  */
 	    case AngelSWI_Reason_ReadC:
-	    case AngelSWI_Reason_IsTTY:
 	    case AngelSWI_Reason_TmpNam:
-	    case AngelSWI_Reason_Remove:
-	    case AngelSWI_Reason_Rename:
 	    case AngelSWI_Reason_System:
 	    case AngelSWI_Reason_EnterSVC:
 	    default:
@@ -684,6 +744,21 @@
 			ARMul_ReadWord (state, addr + 4),
 			ARMul_ReadWord (state, addr + 8));
 	      break;
+
+	    case AngelSWI_Reason_IsTTY:
+	      state->Reg[0] = sim_callback->close (sim_callback,
+						   ARMul_ReadWord (state, addr));
+	      OSptr->ErrorNo = sim_callback->get_errno (sim_callback);
+	      break;
+
+	    case AngelSWI_Reason_Remove:
+	      SWIremove (state,
+			 ARMul_ReadWord (state, addr));
+
+	    case AngelSWI_Reason_Rename:
+	      SWIrename (state,
+			 ARMul_ReadWord (state, addr),
+			 ARMul_ReadWord (state, addr + 4));
 	    }
 	}
       else

[-- Attachment #2: sim-arm-swi.diff --]
[-- Type: text/plain, Size: 5348 bytes --]

2005-09-17  Shaun Jackman  <sjackman@gmail.com>

	* sim/arm/armos.c: Include limits.h
	(unlink): Remove this macro. It is unused in this file and
	conflicts with sim_callback->unlink.
	(PATH_MAX): Define as 1024 if not already defined.
	(ReadFileName): New function.
	(SWIopen): Fix a potential buffer overflow.
	(SWIremove): New function.
	(SWIrename): Ditto.
	(ARMul_OSHandleSWI): Handle the RDP calls SWI_IsTTY,
	SWI_Remove, and SWI_Rename, as well as the RDI calls
	AngelSWI_Reason_IsTTY, AngelSWI_Reason_Remove, and
	AngelSWI_Reason_Rename.

Index: sim/arm/armos.c
===================================================================
RCS file: /cvs/src/src/sim/arm/armos.c,v
retrieving revision 1.22
diff -u -r1.22 armos.c
--- sim/arm/armos.c	12 May 2005 07:36:58 -0000	1.22
+++ sim/arm/armos.c	17 Sep 2005 23:24:51 -0000
@@ -27,6 +27,7 @@
 
 #include <time.h>
 #include <errno.h>
+#include <limits.h>
 #include <string.h>
 #include "targ-vals.h"
 
@@ -34,10 +35,6 @@
 #define TARGET_O_BINARY 0
 #endif
 
-#ifdef __STDC__
-#define unlink(s) remove(s)
-#endif
-
 #ifdef HAVE_UNISTD_H
 #include <unistd.h>		/* For SEEK_SET etc.  */
 #endif
@@ -89,6 +86,9 @@
 #define FOPEN_MAX 64
 #endif
 #define UNIQUETEMPS 256
+#ifndef PATH_MAX
+#define PATH_MAX 1024
+#endif
 
 /* OS private Information.  */
 
@@ -299,22 +299,34 @@
   while (temp != 0);
 }
 
+static int
+ReadFileName(ARMul_State *state, char *buf, ARMword src, size_t n)
+{
+  struct OSblock *OSptr = (struct OSblock *) state->OSptr;
+  char *p = buf;
+
+  while (n--)
+    if ((*p++ = ARMul_SafeReadByte (state, src++)) == '\0')
+      return 0;
+  OSptr->ErrorNo = cb_host_to_target_errno(sim_callback, ENAMETOOLONG);
+  return state->Reg[0] = -1;
+}
+
 static void
 SWIopen (ARMul_State * state, ARMword name, ARMword SWIflags)
 {
   struct OSblock *OSptr = (struct OSblock *) state->OSptr;
-  char dummy[2000];
+  char buf[PATH_MAX];
   int flags;
-  int i;
 
-  for (i = 0; (dummy[i] = ARMul_SafeReadByte (state, name + i)); i++)
-    ;
+  if (ReadFileName(state, buf, name, sizeof buf) == -1)
+    return;
 
   /* Now we need to decode the Demon open mode.  */
   flags = translate_open_mode[SWIflags];
 
   /* Filename ":tt" is special: it denotes stdin/out.  */
-  if (strcmp (dummy, ":tt") == 0)
+  if (strcmp (buf, ":tt") == 0)
     {
       if (flags == TARGET_O_RDONLY) /* opening tty "r" */
 	state->Reg[0] = 0;	/* stdin */
@@ -323,7 +335,7 @@
     }
   else
     {
-      state->Reg[0] = sim_callback->open (sim_callback, dummy, flags);
+      state->Reg[0] = sim_callback->open (sim_callback, buf, flags);
       OSptr->ErrorNo = sim_callback->get_errno (sim_callback);
     }
 }
@@ -403,6 +415,33 @@
   OSptr->ErrorNo = sim_callback->get_errno (sim_callback);
 }
 
+static void
+SWIremove (ARMul_State * state, ARMword path)
+{
+  char buf[PATH_MAX];
+
+  if (ReadFileName(state, buf, path, sizeof buf) != -1)
+  {
+    struct OSblock *OSptr = (struct OSblock *) state->OSptr;
+    state->Reg[0] = sim_callback->unlink (sim_callback, buf);
+    OSptr->ErrorNo = sim_callback->get_errno (sim_callback);
+  }
+}
+
+static void
+SWIrename (ARMul_State * state, ARMword old, ARMword new)
+{
+  char oldbuf[PATH_MAX], newbuf[PATH_MAX];
+
+  if (ReadFileName(state, oldbuf, old, sizeof oldbuf) != -1 &&
+      ReadFileName(state, newbuf, new, sizeof newbuf) != -1)
+  {
+    struct OSblock *OSptr = (struct OSblock *) state->OSptr;
+    state->Reg[0] = sim_callback->rename (sim_callback, oldbuf, newbuf);
+    OSptr->ErrorNo = sim_callback->get_errno (sim_callback);
+  }
+}
+
 /* The emulator calls this routine when a SWI instruction is encuntered.
    The parameter passed is the SWI number (lower 24 bits of the instruction).  */
 
@@ -544,6 +583,30 @@
       state->Emulate = FALSE;
       break;
 
+    case SWI_Remove:
+      if (swi_mask & SWI_MASK_DEMON)
+	SWIremove (state, state->Reg[0]);
+      else
+	unhandled = TRUE;
+      break;
+
+    case SWI_Rename:
+      if (swi_mask & SWI_MASK_DEMON)
+	SWIrename (state, state->Reg[0], state->Reg[1]);
+      else
+	unhandled = TRUE;
+      break;
+
+    case SWI_IsTTY:
+      if (swi_mask & SWI_MASK_DEMON)
+	{
+	  state->Reg[0] = sim_callback->isatty (sim_callback, state->Reg[0]);
+	  OSptr->ErrorNo = sim_callback->get_errno (sim_callback);
+	}
+      else
+	unhandled = TRUE;
+      break;
+
       /* Handle Angel SWIs as well as Demon ones.  */
     case AngelSWI_ARM:
     case AngelSWI_Thumb:
@@ -566,10 +629,7 @@
 	  
 	      /* Unimplemented reason codes.  */
 	    case AngelSWI_Reason_ReadC:
-	    case AngelSWI_Reason_IsTTY:
 	    case AngelSWI_Reason_TmpNam:
-	    case AngelSWI_Reason_Remove:
-	    case AngelSWI_Reason_Rename:
 	    case AngelSWI_Reason_System:
 	    case AngelSWI_Reason_EnterSVC:
 	    default:
@@ -684,6 +744,21 @@
 			ARMul_ReadWord (state, addr + 4),
 			ARMul_ReadWord (state, addr + 8));
 	      break;
+
+	    case AngelSWI_Reason_IsTTY:
+	      state->Reg[0] = sim_callback->close (sim_callback,
+						   ARMul_ReadWord (state, addr));
+	      OSptr->ErrorNo = sim_callback->get_errno (sim_callback);
+	      break;
+
+	    case AngelSWI_Reason_Remove:
+	      SWIremove (state,
+			 ARMul_ReadWord (state, addr));
+
+	    case AngelSWI_Reason_Rename:
+	      SWIrename (state,
+			 ARMul_ReadWord (state, addr),
+			 ARMul_ReadWord (state, addr + 4));
 	    }
 	}
       else

Re: sim/arm/armos.c: IsTTY [PATCH]

[Daniel Jacobowitz]

It only now occurs to me to ask if you have a copyright assignment on
file with the FSF - do you?  I don't see one listed.  Sorry for not
catching this potential problem earlier.

On Sat, Sep 17, 2005 at 05:32:30PM -0600, Shaun Jackman wrote:
> +static int
> +ReadFileName(ARMul_State *state, char *buf, ARMword src, size_t n)

Space before the parens here.

> +{
> +  struct OSblock *OSptr = (struct OSblock *) state->OSptr;
> +  char *p = buf;
> +
> +  while (n--)
> +    if ((*p++ = ARMul_SafeReadByte (state, src++)) == '\0')
> +      return 0;
> +  OSptr->ErrorNo = cb_host_to_target_errno(sim_callback, ENAMETOOLONG);
> +  return state->Reg[0] = -1;

I'd appreciate it if you didn't use the return value of assignment this
way; it's too easy to assume a typo.

> +  if (ReadFileName(state, buf, path, sizeof buf) != -1)
> +  {

Space before parens, braces get indented.

> +  if (ReadFileName(state, oldbuf, old, sizeof oldbuf) != -1 &&
> +      ReadFileName(state, newbuf, new, sizeof newbuf) != -1)

And operators go at the beginning of the line.  Looks odd at first, but
you get used to it.


-- 
Daniel Jacobowitz
CodeSourcery, LLC

Re: sim/arm/armos.c: IsTTY [PATCH]

[Shaun Jackman]

[-- Attachment #1: Type: text/plain, Size: 6320 bytes --]

2005/9/17, Daniel Jacobowitz <drow@false.org>:
> It only now occurs to me to ask if you have a copyright assignment on
> file with the FSF - do you?  I don't see one listed.  Sorry for not
> catching this potential problem earlier.

No, I haven't. I'll reply privately with a fax number if that's
possible. Otherwise, I'll give you my address for mashed-tree
correspondance.

> Space before the parens here.
...
> I'd appreciate it if you didn't use the return value of assignment this
> way; it's too easy to assume a typo.
...
> Space before parens, braces get indented.
...
> And operators go at the beginning of the line.  Looks odd at first, but
> you get used to it.

OK.

Cheers,
Shaun

2005-09-17  Shaun Jackman  <sjackman@gmail.com>

	* sim/arm/armos.c: Include limits.h
	(unlink): Remove this macro. It is unused in this file and
	conflicts with sim_callback->unlink.
	(PATH_MAX): Define as 1024 if not already defined.
	(ReadFileName): New function.
	(SWIopen): Fix a potential buffer overflow.
	(SWIremove): New function.
	(SWIrename): Ditto.
	(ARMul_OSHandleSWI): Handle the RDP calls SWI_IsTTY,
	SWI_Remove, and SWI_Rename, as well as the RDI calls
	AngelSWI_Reason_IsTTY, AngelSWI_Reason_Remove, and
	AngelSWI_Reason_Rename.

Index: sim/arm/armos.c
===================================================================
RCS file: /cvs/src/src/sim/arm/armos.c,v
retrieving revision 1.22
diff -u -r1.22 armos.c
--- sim/arm/armos.c	12 May 2005 07:36:58 -0000	1.22
+++ sim/arm/armos.c	18 Sep 2005 02:16:45 -0000
@@ -27,6 +27,7 @@
 
  #include <time.h>
  #include <errno.h>
+#include <limits.h>
  #include <string.h>
  #include "targ-vals.h"
 
@@ -34,10 +35,6 @@
  #define TARGET_O_BINARY 0
  #endif
 
-#ifdef __STDC__
-#define unlink(s) remove(s)
-#endif
-
  #ifdef HAVE_UNISTD_H
  #include <unistd.h>		/* For SEEK_SET etc.  */
  #endif
@@ -89,6 +86,9 @@
  #define FOPEN_MAX 64
  #endif
  #define UNIQUETEMPS 256
+#ifndef PATH_MAX
+#define PATH_MAX 1024
+#endif
 
  /* OS private Information.  */
 
@@ -299,22 +299,35 @@
   while (temp != 0);
 }
 
+static int
+ReadFileName (ARMul_State * state, char *buf, ARMword src, size_t n)
+{
+  struct OSblock *OSptr = (struct OSblock *) state->OSptr;
+  char *p = buf;
+
+  while (n--)
+    if ((*p++ = ARMul_SafeReadByte (state, src++)) == '\0')
+      return 0;
+  OSptr->ErrorNo = cb_host_to_target_errno(sim_callback, ENAMETOOLONG);
+  state->Reg[0] = -1;
+  return -1;
+}
+
  static void
  SWIopen (ARMul_State * state, ARMword name, ARMword SWIflags)
 {
   struct OSblock *OSptr = (struct OSblock *) state->OSptr;
-  char dummy[2000];
+  char buf[PATH_MAX];
   int flags;
-  int i;
 
-  for (i = 0; (dummy[i] = ARMul_SafeReadByte (state, name + i)); i++)
-    ;
+  if (ReadFileName (state, buf, name, sizeof buf) == -1)
+    return;
 
   /* Now we need to decode the Demon open mode.  */
   flags = translate_open_mode[SWIflags];
 
   /* Filename ":tt" is special: it denotes stdin/out.  */
-  if (strcmp (dummy, ":tt") == 0)
+  if (strcmp (buf, ":tt") == 0)
     {
       if (flags == TARGET_O_RDONLY) /* opening tty "r" */
  	state->Reg[0] = 0;	/* stdin */
@@ -323,7 +336,7 @@
     }
   else
     {
-      state->Reg[0] = sim_callback->open (sim_callback, dummy, flags);
+      state->Reg[0] = sim_callback->open (sim_callback, buf, flags);
       OSptr->ErrorNo = sim_callback->get_errno (sim_callback);
     }
 }
@@ -403,6 +416,33 @@
   OSptr->ErrorNo = sim_callback->get_errno (sim_callback);
 }
 
+static void
+SWIremove (ARMul_State * state, ARMword path)
+{
+  char buf[PATH_MAX];
+
+  if (ReadFileName (state, buf, path, sizeof buf) != -1)
+    {
+      struct OSblock *OSptr = (struct OSblock *) state->OSptr;
+      state->Reg[0] = sim_callback->unlink (sim_callback, buf);
+      OSptr->ErrorNo = sim_callback->get_errno (sim_callback);
+    }
+}
+
+static void
+SWIrename (ARMul_State * state, ARMword old, ARMword new)
+{
+  char oldbuf[PATH_MAX], newbuf[PATH_MAX];
+
+  if (ReadFileName (state, oldbuf, old, sizeof oldbuf) != -1
+      && ReadFileName (state, newbuf, new, sizeof newbuf) != -1)
+    {
+      struct OSblock *OSptr = (struct OSblock *) state->OSptr;
+      state->Reg[0] = sim_callback->rename (sim_callback, oldbuf, newbuf);
+      OSptr->ErrorNo = sim_callback->get_errno (sim_callback);
+    }
+}
+
  /* The emulator calls this routine when a SWI instruction is encuntered.
    The parameter passed is the SWI number (lower 24 bits of the
instruction).  */
 
@@ -544,6 +584,30 @@
       state->Emulate = FALSE;
       break;
 
+    case SWI_Remove:
+      if (swi_mask & SWI_MASK_DEMON)
+	SWIremove (state, state->Reg[0]);
+      else
+	unhandled = TRUE;
+      break;
+
+    case SWI_Rename:
+      if (swi_mask & SWI_MASK_DEMON)
+	SWIrename (state, state->Reg[0], state->Reg[1]);
+      else
+	unhandled = TRUE;
+      break;
+
+    case SWI_IsTTY:
+      if (swi_mask & SWI_MASK_DEMON)
+	{
+	  state->Reg[0] = sim_callback->isatty (sim_callback, state->Reg[0]);
+	  OSptr->ErrorNo = sim_callback->get_errno (sim_callback);
+	}
+      else
+	unhandled = TRUE;
+      break;
+
       /* Handle Angel SWIs as well as Demon ones.  */
     case AngelSWI_ARM:
     case AngelSWI_Thumb:
@@ -566,10 +630,7 @@
 	  
  	      /* Unimplemented reason codes.  */
 	    case AngelSWI_Reason_ReadC:
-	    case AngelSWI_Reason_IsTTY:
 	    case AngelSWI_Reason_TmpNam:
-	    case AngelSWI_Reason_Remove:
-	    case AngelSWI_Reason_Rename:
 	    case AngelSWI_Reason_System:
 	    case AngelSWI_Reason_EnterSVC:
  	    default:
@@ -684,6 +745,21 @@
  			ARMul_ReadWord (state, addr + 4),
  			ARMul_ReadWord (state, addr + 8));
 	      break;
+
+	    case AngelSWI_Reason_IsTTY:
+	      state->Reg[0] = sim_callback->close (sim_callback,
+						   ARMul_ReadWord (state, addr));
+	      OSptr->ErrorNo = sim_callback->get_errno (sim_callback);
+	      break;
+
+	    case AngelSWI_Reason_Remove:
+	      SWIremove (state,
+			 ARMul_ReadWord (state, addr));
+
+	    case AngelSWI_Reason_Rename:
+	      SWIrename (state,
+			 ARMul_ReadWord (state, addr),
+			 ARMul_ReadWord (state, addr + 4));
 	    }
 	}
       else

[-- Attachment #2: sim-arm-swi.diff --]
[-- Type: text/plain, Size: 5381 bytes --]

2005-09-17  Shaun Jackman  <sjackman@gmail.com>

	* sim/arm/armos.c: Include limits.h
	(unlink): Remove this macro. It is unused in this file and
	conflicts with sim_callback->unlink.
	(PATH_MAX): Define as 1024 if not already defined.
	(ReadFileName): New function.
	(SWIopen): Fix a potential buffer overflow.
	(SWIremove): New function.
	(SWIrename): Ditto.
	(ARMul_OSHandleSWI): Handle the RDP calls SWI_IsTTY,
	SWI_Remove, and SWI_Rename, as well as the RDI calls
	AngelSWI_Reason_IsTTY, AngelSWI_Reason_Remove, and
	AngelSWI_Reason_Rename.

Index: sim/arm/armos.c
===================================================================
RCS file: /cvs/src/src/sim/arm/armos.c,v
retrieving revision 1.22
diff -u -r1.22 armos.c
--- sim/arm/armos.c	12 May 2005 07:36:58 -0000	1.22
+++ sim/arm/armos.c	18 Sep 2005 02:16:45 -0000
@@ -27,6 +27,7 @@
 
 #include <time.h>
 #include <errno.h>
+#include <limits.h>
 #include <string.h>
 #include "targ-vals.h"
 
@@ -34,10 +35,6 @@
 #define TARGET_O_BINARY 0
 #endif
 
-#ifdef __STDC__
-#define unlink(s) remove(s)
-#endif
-
 #ifdef HAVE_UNISTD_H
 #include <unistd.h>		/* For SEEK_SET etc.  */
 #endif
@@ -89,6 +86,9 @@
 #define FOPEN_MAX 64
 #endif
 #define UNIQUETEMPS 256
+#ifndef PATH_MAX
+#define PATH_MAX 1024
+#endif
 
 /* OS private Information.  */
 
@@ -299,22 +299,35 @@
   while (temp != 0);
 }
 
+static int
+ReadFileName (ARMul_State * state, char *buf, ARMword src, size_t n)
+{
+  struct OSblock *OSptr = (struct OSblock *) state->OSptr;
+  char *p = buf;
+
+  while (n--)
+    if ((*p++ = ARMul_SafeReadByte (state, src++)) == '\0')
+      return 0;
+  OSptr->ErrorNo = cb_host_to_target_errno(sim_callback, ENAMETOOLONG);
+  state->Reg[0] = -1;
+  return -1;
+}
+
 static void
 SWIopen (ARMul_State * state, ARMword name, ARMword SWIflags)
 {
   struct OSblock *OSptr = (struct OSblock *) state->OSptr;
-  char dummy[2000];
+  char buf[PATH_MAX];
   int flags;
-  int i;
 
-  for (i = 0; (dummy[i] = ARMul_SafeReadByte (state, name + i)); i++)
-    ;
+  if (ReadFileName (state, buf, name, sizeof buf) == -1)
+    return;
 
   /* Now we need to decode the Demon open mode.  */
   flags = translate_open_mode[SWIflags];
 
   /* Filename ":tt" is special: it denotes stdin/out.  */
-  if (strcmp (dummy, ":tt") == 0)
+  if (strcmp (buf, ":tt") == 0)
     {
       if (flags == TARGET_O_RDONLY) /* opening tty "r" */
 	state->Reg[0] = 0;	/* stdin */
@@ -323,7 +336,7 @@
     }
   else
     {
-      state->Reg[0] = sim_callback->open (sim_callback, dummy, flags);
+      state->Reg[0] = sim_callback->open (sim_callback, buf, flags);
       OSptr->ErrorNo = sim_callback->get_errno (sim_callback);
     }
 }
@@ -403,6 +416,33 @@
   OSptr->ErrorNo = sim_callback->get_errno (sim_callback);
 }
 
+static void
+SWIremove (ARMul_State * state, ARMword path)
+{
+  char buf[PATH_MAX];
+
+  if (ReadFileName (state, buf, path, sizeof buf) != -1)
+    {
+      struct OSblock *OSptr = (struct OSblock *) state->OSptr;
+      state->Reg[0] = sim_callback->unlink (sim_callback, buf);
+      OSptr->ErrorNo = sim_callback->get_errno (sim_callback);
+    }
+}
+
+static void
+SWIrename (ARMul_State * state, ARMword old, ARMword new)
+{
+  char oldbuf[PATH_MAX], newbuf[PATH_MAX];
+
+  if (ReadFileName (state, oldbuf, old, sizeof oldbuf) != -1
+      && ReadFileName (state, newbuf, new, sizeof newbuf) != -1)
+    {
+      struct OSblock *OSptr = (struct OSblock *) state->OSptr;
+      state->Reg[0] = sim_callback->rename (sim_callback, oldbuf, newbuf);
+      OSptr->ErrorNo = sim_callback->get_errno (sim_callback);
+    }
+}
+
 /* The emulator calls this routine when a SWI instruction is encuntered.
    The parameter passed is the SWI number (lower 24 bits of the instruction).  */
 
@@ -544,6 +584,30 @@
       state->Emulate = FALSE;
       break;
 
+    case SWI_Remove:
+      if (swi_mask & SWI_MASK_DEMON)
+	SWIremove (state, state->Reg[0]);
+      else
+	unhandled = TRUE;
+      break;
+
+    case SWI_Rename:
+      if (swi_mask & SWI_MASK_DEMON)
+	SWIrename (state, state->Reg[0], state->Reg[1]);
+      else
+	unhandled = TRUE;
+      break;
+
+    case SWI_IsTTY:
+      if (swi_mask & SWI_MASK_DEMON)
+	{
+	  state->Reg[0] = sim_callback->isatty (sim_callback, state->Reg[0]);
+	  OSptr->ErrorNo = sim_callback->get_errno (sim_callback);
+	}
+      else
+	unhandled = TRUE;
+      break;
+
       /* Handle Angel SWIs as well as Demon ones.  */
     case AngelSWI_ARM:
     case AngelSWI_Thumb:
@@ -566,10 +630,7 @@
 	  
 	      /* Unimplemented reason codes.  */
 	    case AngelSWI_Reason_ReadC:
-	    case AngelSWI_Reason_IsTTY:
 	    case AngelSWI_Reason_TmpNam:
-	    case AngelSWI_Reason_Remove:
-	    case AngelSWI_Reason_Rename:
 	    case AngelSWI_Reason_System:
 	    case AngelSWI_Reason_EnterSVC:
 	    default:
@@ -684,6 +745,21 @@
 			ARMul_ReadWord (state, addr + 4),
 			ARMul_ReadWord (state, addr + 8));
 	      break;
+
+	    case AngelSWI_Reason_IsTTY:
+	      state->Reg[0] = sim_callback->close (sim_callback,
+						   ARMul_ReadWord (state, addr));
+	      OSptr->ErrorNo = sim_callback->get_errno (sim_callback);
+	      break;
+
+	    case AngelSWI_Reason_Remove:
+	      SWIremove (state,
+			 ARMul_ReadWord (state, addr));
+
+	    case AngelSWI_Reason_Rename:
+	      SWIrename (state,
+			 ARMul_ReadWord (state, addr),
+			 ARMul_ReadWord (state, addr + 4));
 	    }
 	}
       else

Re: sim/arm/armos.c: IsTTY [PATCH]

[Daniel Jacobowitz]

On Sat, Sep 17, 2005 at 08:40:14PM -0600, Shaun Jackman wrote:
> 2005/9/17, Daniel Jacobowitz <drow@false.org>:
> > It only now occurs to me to ask if you have a copyright assignment on
> > file with the FSF - do you?  I don't see one listed.  Sorry for not
> > catching this potential problem earlier.
> 
> No, I haven't. I'll reply privately with a fax number if that's
> possible. Otherwise, I'll give you my address for mashed-tree
> correspondance.

I'll follow up off-list for this bit.

> > Space before the parens here.

Missed one of these:

> +  OSptr->ErrorNo = cb_host_to_target_errno(sim_callback, ENAMETOOLONG);

No need to repost the patch for this.  Otherwise, it looks fine to me.


-- 
Daniel Jacobowitz
CodeSourcery, LLC

Re: sim/arm/armos.c: IsTTY [PATCH]

[Shaun Jackman]

2005/9/17, Daniel Jacobowitz <drow@false.org>:
> > I've contacted the FSF clerk.
>
> Thanks.  It may take a little while, but they've been fairly prompt
> lately.  Probably less delay than it took to get the patch reviewed in
> the first place.

My copyright assignment is complete! Please apply the patch from
Message-ID: <7f45d939050917194069e70603@mail.gmail.com>.

Cheers,
Shaun

Re: sim/arm/armos.c: IsTTY [PATCH]

[Shaun Jackman]

Ping?

> 2005-09-17  Shaun Jackman  <sjackman@gmail.com>
>
>         * sim/arm/armos.c: Include limits.h
>         (unlink): Remove this macro. It is unused in this file and
>         conflicts with sim_callback->unlink.
>         (PATH_MAX): Define as 1024 if not already defined.
>         (ReadFileName): New function.
>         (SWIopen): Fix a potential buffer overflow.
>         (SWIremove): New function.
>         (SWIrename): Ditto.
>         (ARMul_OSHandleSWI): Handle the RDP calls SWI_IsTTY,
>         SWI_Remove, and SWI_Rename, as well as the RDI calls
>         AngelSWI_Reason_IsTTY, AngelSWI_Reason_Remove, and
>         AngelSWI_Reason_Rename.

Re: sim/arm/armos.c: IsTTY [PATCH]

[Daniel Jacobowitz]

On Wed, Oct 19, 2005 at 10:12:52AM -0600, Shaun Jackman wrote:
> Ping?
> 
> > 2005-09-17  Shaun Jackman  <sjackman@gmail.com>
> >
> >         * sim/arm/armos.c: Include limits.h
> >         (unlink): Remove this macro. It is unused in this file and
> >         conflicts with sim_callback->unlink.
> >         (PATH_MAX): Define as 1024 if not already defined.
> >         (ReadFileName): New function.
> >         (SWIopen): Fix a potential buffer overflow.
> >         (SWIremove): New function.
> >         (SWIrename): Ditto.
> >         (ARMul_OSHandleSWI): Handle the RDP calls SWI_IsTTY,
> >         SWI_Remove, and SWI_Rename, as well as the RDI calls
> >         AngelSWI_Reason_IsTTY, AngelSWI_Reason_Remove, and
> >         AngelSWI_Reason_Rename.

Sorry for the delay.  I've applied this.

-- 
Daniel Jacobowitz
CodeSourcery, LLC

Re: sim/arm/armos.c: IsTTY [PATCH]

[Shaun Jackman]

2005/11/16, Daniel Jacobowitz <drow@false.org>:
> Sorry for the delay.  I've applied this.

No worries. Thanks, Daniel!

Cheers,
Shaun

Re: sim/arm/armos.c: IsTTY [PATCH]

[Eli Zaretskii]

> Date: Sat, 17 Sep 2005 18:37:28 -0400
> From: Daniel Jacobowitz <drow@false.org>
> Cc: Richard Earnshaw <rearnsha@gcc.gnu.org>, gdb-patches@sources.redhat.com
> 
>     Usage Note: Don't use FILENAME_MAX as the size of an array in which
> to store a file name! You can't possibly make an array that big! Use
> dynamic allocation (see section 3.2 Allocating Storage For Program
> Data) instead. 

That's a very strange advice, especially since there's gobs of code
out there that define arrays like that to store file names.  What's
the value of FILENAME_MAX on those systems where a file name can have
unlimited length?  Is it really larger than a typical stack
limitation?

> I'd recommend following Richard's suggestion: use PATH_MAX, if it's not
> there, default to 1K.

FWIW, I hate those arbitrary 1K definitions.

Re: sim/arm/armos.c: IsTTY [PATCH]

[Daniel Jacobowitz]

On Sun, Sep 18, 2005 at 06:32:25AM +0300, Eli Zaretskii wrote:
> > Date: Sat, 17 Sep 2005 18:37:28 -0400
> > From: Daniel Jacobowitz <drow@false.org>
> > Cc: Richard Earnshaw <rearnsha@gcc.gnu.org>, gdb-patches@sources.redhat.com
> > 
> >     Usage Note: Don't use FILENAME_MAX as the size of an array in which
> > to store a file name! You can't possibly make an array that big! Use
> > dynamic allocation (see section 3.2 Allocating Storage For Program
> > Data) instead. 
> 
> That's a very strange advice, especially since there's gobs of code
> out there that define arrays like that to store file names.  What's
> the value of FILENAME_MAX on those systems where a file name can have
> unlimited length?  Is it really larger than a typical stack
> limitation?

It's looks like it has since been toned down as a matter of
practicality, but yes: I believe that at one point it was INT_MAX.
I.E. Much Too Big.

Ah, yes, Hurd throttles it to 1K in response to a limitation in their
RPC mechanism.  But the developers were making noise about unthrottling
it again someday.

-- 
Daniel Jacobowitz
CodeSourcery, LLC

Re: sim/arm/armos.c: IsTTY [PATCH]

[Mark Kettenis]

> Date: Sat, 17 Sep 2005 23:44:32 -0400
> From: Daniel Jacobowitz <drow@false.org>
> 
> On Sun, Sep 18, 2005 at 06:32:25AM +0300, Eli Zaretskii wrote:
> > > Date: Sat, 17 Sep 2005 18:37:28 -0400
> > > From: Daniel Jacobowitz <drow@false.org>
> > > Cc: Richard Earnshaw <rearnsha@gcc.gnu.org>, gdb-patches@sources.redhat.com
> > > 
> > >     Usage Note: Don't use FILENAME_MAX as the size of an array in which
> > > to store a file name! You can't possibly make an array that big! Use
> > > dynamic allocation (see section 3.2 Allocating Storage For Program
> > > Data) instead. 
> > 
> > That's a very strange advice, especially since there's gobs of code
> > out there that define arrays like that to store file names.  What's
> > the value of FILENAME_MAX on those systems where a file name can have
> > unlimited length?  Is it really larger than a typical stack
> > limitation?
> 
> It's looks like it has since been toned down as a matter of
> practicality, but yes: I believe that at one point it was INT_MAX.
> I.E. Much Too Big.
> 
> Ah, yes, Hurd throttles it to 1K in response to a limitation in their
> RPC mechanism.  But the developers were making noise about unthrottling
> it again someday.

FWIW, the correct way to deal with these issues is to dynamically
allocate the buffer, making the buffer bigger if it fails because the
buffer is too small:

  #ifndef _POSIX_PATH_MAX
  #define _POSIX_PATH_MAX 256
  #endif

  size_t len = _POSIX_PATH_MAX;
  char *buf = xmalloc(len);
  while (foo(..., buf, len) == -1 && errno == ENAMETOOLONG)
    {
      len *= 2;
      buf = xrealloc(buf, len);
    }

I used _POSIX_PATH_MAX here, but you could probably just as well
hardcode a sensible value.  Don't forget to free the buffer once
you're done with it ;-).

Mark

Re: sim/arm/armos.c: IsTTY [PATCH]

[Eli Zaretskii]

> Date: Sun, 18 Sep 2005 11:03:41 +0200 (CEST)
> From: Mark Kettenis <mark.kettenis@xs4all.nl>
> CC: eliz@gnu.org, sjackman@gmail.com, rearnsha@gcc.gnu.org,
>         gdb-patches@sources.redhat.com
> 
> FWIW, the correct way to deal with these issues is to dynamically
> allocate the buffer, making the buffer bigger if it fails because the
> buffer is too small:
> 
>   #ifndef _POSIX_PATH_MAX
>   #define _POSIX_PATH_MAX 256
>   #endif
> 
>   size_t len = _POSIX_PATH_MAX;
>   char *buf = xmalloc(len);
>   while (foo(..., buf, len) == -1 && errno == ENAMETOOLONG)
>     {
>       len *= 2;
>       buf = xrealloc(buf, len);
>     }
> 
> I used _POSIX_PATH_MAX here, but you could probably just as well
> hardcode a sensible value.  Don't forget to free the buffer once
> you're done with it ;-).

Forgive my bluntness, but if such a simple job requires such
complicated code, and with caveats on top of that, it's a clear sign
of a botched API, worthy of a certain company from Redmond!