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

[Tom Tromey <tromey at redhat dot com>]

>>>>> "Phil" == Phil Muldoon <pmuldoon@redhat.com> writes:

Phil> This patch adds an optional length parameter to the
Phil> value.string() method.

This needs a documentation change as well.

Also, please update language.h to document the new meaning of the
length argument to la_get_string.  The comment before c_get_string
also needs this.

Phil> +      if ( *length >= 0 )

Extra spaces after "(" and before ")" here.

Phil> +      /* Copy length to orig_len, as read_string will write the number
Phil> +	 of characters 'read' to length.  */
Phil> +      int orig_len = *length;

You don't need this.  Just pass *length to read_string.  The right
thing will happen.


I think we should not strip a trailing \0 in the case that a length
was requested.

Consider a library that uses counted strings but does not terminate
them with \0.  In this case, it seems to me that it would be
impossible to read such a string with a trailing \0.

In the other case, when the length is found by searching for a \0, it
is still appropriate to strip the trailing \0.

So, please implement this, and make sure this behavior is documented.


The rest looks good to me.

Tom