Re: [python][patch] Add options length parameter to value.string(...)

[Phil Muldoon <pmuldoon@redhat.com>]

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

Thiago Jung Bauermann wrote:
> I looked at the patch and it seems good. I have just a few nits, I hope
> you will put up with me just a tad bit more. :-)
>
>   

Thanks for the swift review.

>>  static PyObject *
>>  valpy_string (PyObject *self, PyObject *args, PyObject *kw)
>>  {
>>     
>
> This function has the following comment above it:
>
> /* Implementation of gdb.Value.string ([encoding] [, errors]) -> string
>    Return Unicode string with value contents.  If ENCODING is not given,
>    the string is assumed to be encoded in the target's charset.  */
>
> Would you mind adding "[, length] to the prototype?
>   

Ack, sorry.  Added.

>> +  gdb_test "python print st.string (length = 0)" "" "Test string (length = 0) is empty"
>>     
>
> Does using "" as test pattern really test for an empty line? I know that
> gdb_test puts an EOL marker right after the pattern, but it doesn't put
> any required pattern before the pattern...
>   

Altered the test. Thanks for the tips on irc on doing this.

Patch attached.

Regards

Phil


[-- Attachment #2: valpy_variable_string_fetch.patch --]
[-- Type: text/x-patch, Size: 10862 bytes --]

diff --git a/gdb/c-lang.c b/gdb/c-lang.c
index 8b5410f..d03a88f 100644
--- a/gdb/c-lang.c
+++ b/gdb/c-lang.c
@@ -183,16 +183,20 @@ c_printstr (struct ui_file *stream, const gdb_byte *string,
 }
 
 /* Obtain a C string from the inferior storing it in a newly allocated
-   buffer in BUFFER, which should be freed by the caller.  The string is
-   read until a null character is found. If VALUE is an array with known
-   length, the function will not read past the end of the array.  LENGTH
-   will contain the size of the string in bytes (not counting the null
-   character).
-
-   Assumes strings are terminated by a null character.  The size of a character
-   is determined by the length of the target type of the pointer or array.
-   This means that a null byte present in a multi-byte character will not
-   terminate the string unless the whole character is null.
+   buffer in BUFFER, which should be freed by the caller.  If length
+   is specified at -1, the string is read until a null character is
+   found, otherwise the string is read to the length specified.  
+   If VALUE is an array with a known length, the function will not
+   read past the end of the array.  LENGTH will contain the size of
+   the string in bytes.  (If a length of -1 is specified, the length
+   returned will not include the null character).
+
+   In the case of length specified as -1, assume strings are
+   terminated by a null character.  The size of a character is
+   determined by the length of the target type of the pointer or
+   array.  This means that a null byte present in a multi-byte
+   character will not terminate the string unless the whole character
+   is null.
 
    CHARSET is always set to the target charset.  */
 
@@ -204,6 +208,7 @@ c_get_string (struct value *value, gdb_byte **buffer, int *length,
   unsigned int fetchlimit;
   struct type *type = check_typedef (value_type (value));
   struct type *element_type = TYPE_TARGET_TYPE (type);
+  int req_length = *length;
 
   if (element_type == NULL)
     goto error;
@@ -249,12 +254,17 @@ c_get_string (struct value *value, gdb_byte **buffer, int *length,
       int i;
       const gdb_byte *contents = value_contents (value);
 
-      /* Look for a null character.  */
-      for (i = 0; i < fetchlimit; i++)
-	if (extract_unsigned_integer (contents + i * width, width) == 0)
-	  break;
-
-      /* I is now either the number of non-null characters, or FETCHLIMIT.  */
+      /* If a length is specified, use that.  */
+      if (*length >= 0)
+	i = *length;
+      else
+	/* Otherwise, look for a null character.  */
+	for (i = 0; i < fetchlimit; i++)
+	  if (extract_unsigned_integer (contents + i * width, width) == 0)
+	    break;
+      
+      /* I is now either a user-defined length, the number of non-null
+	 characters, or FETCHLIMIT.  */      
       *length = i * width;
       *buffer = xmalloc (*length);
       memcpy (*buffer, contents, *length);
@@ -262,7 +272,7 @@ c_get_string (struct value *value, gdb_byte **buffer, int *length,
     }
   else
     {
-      err = read_string (value_as_address (value), -1, width, fetchlimit,
+      err = read_string (value_as_address (value), *length, width, fetchlimit,
 			 buffer, length);
       if (err)
 	{
@@ -272,10 +282,15 @@ c_get_string (struct value *value, gdb_byte **buffer, int *length,
 	}
     }
 
-  /* If the last character is null, subtract it from LENGTH.  */
-  if (*length > 0
-      && extract_unsigned_integer (*buffer + *length - width, width) == 0)
-    *length -= width;
+  /* If the LENGTH is specified at -1, we want to return the string
+     length up to the terminating null character.  If an actual length
+     was specified, we want to return the length of exactly what was
+     read.  */
+  if (req_length == -1)
+    /* If the last character is null, subtract it from LENGTH.  */
+    if (*length > 0
+	&& extract_unsigned_integer (*buffer + *length - width, width) == 0)
+      *length -= width;
 
   *charset = target_charset ();
 
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 8509ecc..c7091a6 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -18442,7 +18442,7 @@ The result @code{bar} will be a @code{gdb.Value} object holding the
 value pointed to by @code{foo}.
 @end defmethod
 
-@defmethod Value string @r{[}encoding@r{]} @r{[}errors@r{]}
+@defmethod Value string @r{[}encoding@r{]} @r{[}errors@r{]} @r{[}length@r{]}
 If this @code{gdb.Value} represents a string, then this method
 converts the contents to a Python string.  Otherwise, this method will
 throw an exception.
@@ -18453,7 +18453,9 @@ language.
 
 For C-like languages, a value is a string if it is a pointer to or an
 array of characters or ints.  The string is assumed to be terminated
-by a zero of the appropriate width.
+by a zero of the appropriate width.  However if the optional length
+argument is given, the string will be converted beyond any embedded
+nulls up to the length specified.
 
 If the optional @var{encoding} argument is given, it must be a string
 naming the encoding of the string in the @code{gdb.Value}, such as
@@ -18467,6 +18469,9 @@ will be used, if the current language is able to supply one.
 
 The optional @var{errors} argument is the same as the corresponding
 argument to Python's @code{string.decode} method.
+
+If the optional @var{length} argumen is given, the string will be
+fetched and converted to the given length.
 @end defmethod
 @end table
 
diff --git a/gdb/language.h b/gdb/language.h
index 85826fd..97b93d6 100644
--- a/gdb/language.h
+++ b/gdb/language.h
@@ -283,10 +283,14 @@ struct language_defn
     int (*la_pass_by_reference) (struct type *type);
 
     /* Obtain a string from the inferior, storing it in a newly allocated
-       buffer in BUFFER, which should be freed by the caller.  LENGTH will
-       hold the size in bytes of the string (only actual characters, excluding
-       an eventual terminating null character).  CHARSET will hold the encoding
-       used in the string.  */
+       buffer in BUFFER, which should be freed by the caller.  If LENGTH
+       is specified at -1, the string is read until a null character is
+       found -  otherwise the string is read to the length specified.
+       On completion, LENGTH will hold the size in bytes of the string.
+       If a LENGTH of -1 was specified it will count only actual
+       characters, excluding any eventual terminating null character.
+       Otherwise LENGTH will equal all characters - including any nulls.
+       CHARSET will hold the encoding used in the string.  */
     void (*la_get_string) (struct value *value, gdb_byte **buffer, int *length,
 			  const char **charset);
 
diff --git a/gdb/python/python-value.c b/gdb/python/python-value.c
index 2eaf15f..743e6a6 100644
--- a/gdb/python/python-value.c
+++ b/gdb/python/python-value.c
@@ -190,13 +190,16 @@ valpy_get_type (PyObject *self, void *closure)
   return obj->type;
 }
 
-/* Implementation of gdb.Value.string ([encoding] [, errors]) -> string
-   Return Unicode string with value contents.  If ENCODING is not given,
-   the string is assumed to be encoded in the target's charset.  */
+/* Implementation of gdb.Value.string ([encoding] [, errors] 
+   [, length]) -> string.  Return Unicode string with value contents.
+   If ENCODING is not given, the string is assumed to be encoded in
+   the target's charset.  If LENGTH is provided, only fetch string to
+   the length provided.  */
+
 static PyObject *
 valpy_string (PyObject *self, PyObject *args, PyObject *kw)
 {
-  int length, ret = 0;
+  int length = -1, ret = 0;
   gdb_byte *buffer;
   struct value *value = ((value_object *) self)->value;
   volatile struct gdb_exception except;
@@ -205,10 +208,10 @@ valpy_string (PyObject *self, PyObject *args, PyObject *kw)
   const char *errors = NULL;
   const char *user_encoding = NULL;
   const char *la_encoding = NULL;
-  static char *keywords[] = { "encoding", "errors" };
+  static char *keywords[] = { "encoding", "errors", "length" };
 
-  if (!PyArg_ParseTupleAndKeywords (args, kw, "|ss", keywords,
-				    &user_encoding, &errors))
+  if (!PyArg_ParseTupleAndKeywords (args, kw, "|ssi", keywords,
+				    &user_encoding, &errors, &length))
     return NULL;
 
   TRY_CATCH (except, RETURN_MASK_ALL)
@@ -975,7 +978,7 @@ static PyMethodDef value_object_methods[] = {
   { "cast", valpy_cast, METH_VARARGS, "Cast the value to the supplied type." },
   { "dereference", valpy_dereference, METH_NOARGS, "Dereferences the value." },
   { "string", (PyCFunction) valpy_string, METH_VARARGS | METH_KEYWORDS,
-    "string ([encoding] [, errors]) -> string\n\
+    "string ([encoding] [, errors] [, length]) -> string\n\
 Return Unicode string representation of the value." },
   {NULL}  /* Sentinel */
 };
diff --git a/gdb/testsuite/gdb.python/python-value.c b/gdb/testsuite/gdb.python/python-value.c
index 092c520..f3d6284 100644
--- a/gdb/testsuite/gdb.python/python-value.c
+++ b/gdb/testsuite/gdb.python/python-value.c
@@ -44,6 +44,8 @@ main (int argc, char *argv[])
   struct s s;
   union u u;
   PTR x = &s;
+  char st[17] = "divide et impera";
+  char nullst[17] = "divide\0et\0impera";
 
   s.a = 3;
   s.b = 5;
diff --git a/gdb/testsuite/gdb.python/python-value.exp b/gdb/testsuite/gdb.python/python-value.exp
index 3421406..59d13c6 100644
--- a/gdb/testsuite/gdb.python/python-value.exp
+++ b/gdb/testsuite/gdb.python/python-value.exp
@@ -234,6 +234,25 @@ proc test_value_in_inferior {} {
 
   # Test address attribute
   gdb_test "python print 'result =', arg0.address" "= 0x\[\[:xdigit:\]\]+" "Test address attribute"
+
+  # Test string fetches,  both partial and whole.
+  gdb_test "print st" "\"divide et impera\""
+  gdb_py_test_silent_cmd "python st = gdb.history (0)" "get value from history" 1
+  gdb_test "python print st.string ()"  "divide et impera"  "Test string with no length"
+  gdb_test "python print st.string (length = -1)" "divide et impera" "Test string (length = -1) is all of the string"
+  gdb_test "python print st.string (length = 6)" "divide"
+  gdb_test "python print \"---\"+st.string (length = 0)+\"---\"" "------" "Test string (length = 0) is empty"
+  gdb_test "python print len(st.string (length = 0))" "0" "Test length is 0"
+
+
+  # Fetch a string that has embedded nulls.
+  gdb_test "print nullst" "\"divide\\\\000et\\\\000impera\".*"
+  gdb_py_test_silent_cmd "python nullst = gdb.history (0)" "get value from history" 1
+  gdb_test "python print nullst.string ()" "divide" "Test string to first null"
+  # Python cannot print strings that contain the null (\0) character.
+  # For the purposes of this test, use repr()
+  gdb_py_test_silent_cmd "python nullst = nullst.string (length = 9)" "get string beyond null" 1
+  gdb_test "python print repr(nullst)" "u'divide\\\\x00et'"
 }
 
 proc test_value_after_death {} {