Re: [RFA] Add la_getstr member to language_defn

[Tom Tromey <tromey at redhat dot com>]

>>>>> "Thiago" == Thiago Jung Bauermann <bauerman@br.ibm.com> writes:

Thiago> Right. Here's the new version. It also uses c_get_string for Ada and
Thiago> minimal...

I read it a little more closely and I'm afraid I have a couple more
comments.  Mostly they are little formatting nits, nothing important.

If you agree with the more substantive comments, then I think this
patch is ok with the suggested changes -- no need for another review.
If you don't agree, let's discuss it.

Thiago> +void
Thiago> +c_get_string (struct value *value, gdb_byte **buffer, int *length,
Thiago> +	      const char **charset)
Thiago> +{
Thiago> +  int err, width;
Thiago> +  unsigned int fetchlimit;
Thiago> +  struct type *type = value_type (value);

I think this should probably use check_typedef.  Otherwise it seems
like we would not extract some strings properly.  E.g., consider a
case like "typedef char *string;" ... what will happen?

Thiago> +  else if (TYPE_CODE (value_type (value)) == TYPE_CODE_PTR)

This should use 'type' instead of calling value_type again.

Thiago> +	if (extract_unsigned_integer (contents + i*width, width) == 0)

Need spaces around the "*".

Thiago> +      /* i is now either the number of non-null characters, or fetchlimit.  */

Use "I", not "i" .. a tiny nit for the GNU style, where the value of a
variable is referred to by uppercasing the name.

Thiago> +      *length = i*width;

Spaces.

Thiago> +  /* If the last character is null, subtract it from length.  */
Thiago> +  if (extract_unsigned_integer (*buffer + *length - width, width) == 0)
Thiago> +      *length -= width;

This second line looks like it has too much indentation.

Also, I suspect this should start "if (*length > 0 &&".
Otherwise, it seems like it will read memory before *BUFFER when
*LENGTH==0.

Thiago> +error:

Emacs claims that labels are indented one space in the GNU style.
I didn't see this in the standards manual though.

Thiago> +++ b/gdb/value.h
[...]
Thiago> +extern char * type_to_string (struct type *type);

Extra space after the "*".

thanks for persevering,
Tom