RFA: gloss component patch

[Ben Elliston <bje at redhat dot com>]

I would like to submit the following patch the gloss component.
Currently, there is some confusion surrounding the semantics of
gloss32::get_string.  The comments say that it does not handle NUL
termination, but the code *does*.  Thus, a target program issuing:

      write (3, #some-pointer#, 4096);

will fail to behave properly when the buffer contains a NUL.

I have cleaned up the semantics a bit -- now there are two
get_string() methods: one with a length parameter and one without.  If
a length parameter is supplied, exactly those many bytes are read from
target memory.  If it is omitted, the buffer is considered to be NUL
terminated.  The patch appears below -- okay to commit?


2001-09-26  Ben Elliston  <bje@redhat.com>

	* gloss.h (class gloss32): Add new get_string method which takes
	no length parameter.  Memory is read until a NUL is reached.
	* gloss.cxx (gloss32::get_string): Call get_string() with length 0.
	(gloss32::get_string): Handle both fixed length and NUL-terminated
	strings (signified by length = 0).
	(gloss32::write): Improve verbose message.
	(gloss32::do_sys_open): Improve failure mode for get_string().

Index: gloss.cxx
===================================================================
RCS file: /cvs/cvsfiles/devo/sid/component/gloss/gloss.cxx,v
retrieving revision 1.36
diff -u -r1.36 gloss.cxx
--- gloss.cxx	2001/09/24 06:42:04	1.36
+++ gloss.cxx	2001/09/26 01:57:35
@@ -148,10 +148,20 @@
 
 // Memory access methods.
 
+// Read a NUL-terminated string from memory.
 bool
-gloss32::get_string(address32 address, string& value, unsigned length)
+gloss32::get_string (address32 address, string& value)
 {
-  if (! this->cpu_memory_bus)
+  return get_string (address, value, 0);
+}
+
+// Calling get_string with length = 0 indicates that there is no
+// imposed length limit; read from memory until a NUL is encountered.
+
+bool
+gloss32::get_string (address32 address, string& value, unsigned length)
+{
+  if (! cpu_memory_bus)
     {
       cerr << "*** CPU memory bus not configured!" << endl;
       return false;
@@ -163,33 +173,30 @@
 	   << make_numeric_attribute (address, ios::hex | ios::showbase) << ": ";
     }
 
-  for (unsigned i = 0; i < length; i++)
+  for (unsigned i = 0; (i < length || length == 0); i++)
     {
-      little_int_1 byte; // equivalent to big_int_1
-      while(true)
+      char c;
+      little_int_1 byte;
+
+      bus::status s = cpu_memory_bus->read (address, byte);
+      if (s != bus::ok) 
 	{
-	  bus::status s = this->cpu_memory_bus->read(address, byte);
-	  if (s == bus::ok) 
-	    break;
-	  else 
-	    {
-	      if (verbose_p)
-		cerr << "failed" << endl;
-	      return false;
-	    }
+	  if (verbose_p)
+	    cerr << "failed" << endl;
+	  return false;
 	}
+
+      c = byte;
+      if (length == 0 && c == '\0')
+	break;
 
-      char c = byte;
       value += c;
+      address++;
+      
       if (verbose_p)
 	cerr << "[" << c << "]";
-
-      address = address + 1;
-
-      if (c == '\0')
-	break;
     }
-  
+
   if (verbose_p)
     cerr << endl;
   return true;
@@ -211,10 +218,11 @@
            << ": ";
     }
   
-  for (unsigned i=0; i < value.size(); i++)
+  for (unsigned i = 0; i < value.size(); i++)
     {
       char c = value[i];
-      little_int_1 byte = c; // equivalent to big_int_1
+      little_int_1 byte = c;
+
       while (true)
 	{
 	  bus::status s = this->cpu_memory_bus->write(address, byte);
@@ -227,11 +235,10 @@
 	      return false;
 	    }
 	}
-
+      address++;
+      
       if (verbose_p)
 	cerr << "[" << c << "]";
-
-      address = address + 1;
     }
 
   if (verbose_p)
@@ -978,7 +985,12 @@
   get_int_argument(2, open_flags);
   get_int_argument(3, mode);
 
-  get_string(str_ptr, filename, 100);
+  if (!get_string (str_ptr, filename))
+    {
+      set_error_result (newlib::eFault);
+      set_int_result (-1);
+      return;
+    }
 
   if (!target_to_host_open_flags (open_flags, flags))
     {
@@ -1314,7 +1326,7 @@
 		  && host_fd == fd);
 
   if (verbose_p)
-    cerr << "*** write(" << fd << "," << addr << "," << len << ")" << endl;
+    cerr << "*** write(" << fd << ", 0x" << hex << addr << ", " << dec << len << ")" << endl;
 
   if (host_fd == -1)
     {
Index: gloss.h
===================================================================
RCS file: /cvs/cvsfiles/devo/sid/component/gloss/gloss.h,v
retrieving revision 1.19
diff -u -r1.19 gloss.h
--- gloss.h	2001/09/04 03:39:46	1.19
+++ gloss.h	2001/09/26 01:57:36
@@ -118,9 +118,11 @@
   bool set_word(address32 address, int32 value);
 
   // Read/write a string in memory.
-  // Note that this is a string of bytes, not a nul-terminated string.
-  bool get_string(address32 address, string& value, unsigned length);
+  // The first version of get_string reads a null-terminated string.
+  // The second version reads a fixed number of bytes from memory.
+  bool get_string(address32 address, string& value);
   bool set_string(address32 address, const string& value);
+  bool get_string(address32 address, string& value, unsigned length);
 
   // Get the value of the cpu's program counter.
   virtual bool get_pc(address32& value);