Re: [target-description] enum and typed fields in struct

[Yao Qi <yao at codesourcery dot com>]

On 06/03/2011 07:05 PM, Wei-cheng Wang wrote:
> Would it be nice if âprint $cpsrâ shows
> `$1 = {[ #4 T C Z ], M = User, IT_cond = 0, GE = 0, DNM = 0}'
> instead of `$1 = 1610612784' ?
> 

I am not the people to approve or reject this, but I like this feature.
 Thanks.

Usually, we need to post ChangeLog entry here to reflect your changes.

I am not familiar with target-description code.  Some comments on coding
style and indent.

> 
> #	modified:   gdb/features/gdb-target.dtd
> #	modified:   gdb/gdbtypes.c
> #	modified:   gdb/gdbtypes.h
> #	modified:   gdb/target-descriptions.c
> #	modified:   gdb/target-descriptions.h
> #	modified:   gdb/xml-tdesc.c

> @@ -630,13 +631,16 @@ tdesc_gdb_type (struct gdbarch *gdbarch, struct
> tdesc_type *tdesc_type)
>  	     VEC_iterate (tdesc_type_field, tdesc_type->u.u.fields, ix, f);
>  	     ix++)
>  	  {
> +	    struct field *fld;
> +	    struct type *field_type;
> +	    int bitsize, total_size;
> +
> +	    /* This code should backward compatible with old tdesc.
                               ^ be
I am not a native English speaker, but I think there is a 'be' needed
after `should'.
> +	       For non-sized struct with typed-fields,
The `newline' is not needed here.
> +	       the size of struct should be the sum of size of all fields.
> +	       append_composite_type_field()  */
>  	    if (f->type == NULL)
>  	      {
> -		/* Bitfield.  */
> -		struct field *fld;
> -		struct type *field_type;
> -		int bitsize, total_size;
> -
>  		/* This invariant should be preserved while creating
>  		   types.  */
>  		gdb_assert (tdesc_type->u.u.size != 0);
> @@ -647,28 +651,42 @@ tdesc_gdb_type (struct gdbarch *gdbarch, struct
> tdesc_type *tdesc_type)
> 
>  		fld = append_composite_type_field_raw (type, xstrdup (f->name),
>  						       field_type);
> -
> -		/* For little-endian, BITPOS counts from the LSB of
> -		   the structure and marks the LSB of the field.  For
> -		   big-endian, BITPOS counts from the MSB of the
> -		   structure and marks the MSB of the field.  Either
> -		   way, it is the number of bits to the "left" of the
> -		   field.  To calculate this in big-endian, we need
> -		   the total size of the structure.  */
> -		bitsize = f->end - f->start + 1;
> -		total_size = tdesc_type->u.u.size * TARGET_CHAR_BIT;
> -		if (gdbarch_bits_big_endian (gdbarch))
> -		  FIELD_BITPOS (fld[0]) = total_size - f->start - bitsize;
> -		else
> -		  FIELD_BITPOS (fld[0]) = f->start;
> -		FIELD_BITSIZE (fld[0]) = bitsize;
>  	      }
>  	    else
>  	      {
>  		field_type = tdesc_gdb_type (gdbarch, f->type);
> -		append_composite_type_field (type, xstrdup (f->name),
> +		fld = append_composite_type_field (type, xstrdup (f->name),
>  					     field_type);
>  	      }
> +
> +	    /* This will turn all fields into bitfileds */

Please use "." at the end your comment, with two spaces.

> +	    if (f->start == -1)
> +	      {
> +		if (TYPE_NFIELDS (type) > 1)
> +		  f->start = (FIELD_BITPOS (fld[-1]) + FIELD_BITSIZE (fld[-1]));
> +		else
> +		  f->start = 0;
> +	      }
> +	    if (f->end == -1)
> +	      {
> +		f->end = f->start - 1 + TYPE_LENGTH (field_type)
> +					* TARGET_CHAR_BIT;

The indent doesn't look right.  You have a look at GNU Coding Standard.
http://www.gnu.org/prep/standards/standards.html#Writing-C

>  	if (tdesc_type->u.u.size != 0)
> @@ -719,6 +737,27 @@ tdesc_gdb_type (struct gdbarch *gdbarch, struct
> tdesc_type *tdesc_type)
> 
>  	return type;
>        }
> +
> +    case TDESC_TYPE_ENUM:
> +      {
> +	struct tdesc_type_flag *f;
> +	int ix;
> +
The indent looks not right.  Please reference
http://www.gnu.org/prep/standards/standards.html#Formatting

> +	type = arch_type (gdbarch, TYPE_CODE_ENUM,
> +			  tdesc_type->u.f.size, tdesc_type->name);
> +	TYPE_UNSIGNED (type) = 1;
> +	for (ix = 0;
> +	     VEC_iterate (tdesc_type_flag, tdesc_type->u.f.flags, ix, f);
> +	     ix++)
> +	  {
> +	    struct field *fld;
> +	    fld = append_composite_type_field_raw (type,
> +					xstrdup (f->name), NULL);
> +	    FIELD_BITPOS (fld[0]) = f->start;
> +	  }
> +
> +	return type;
> +      }
>      }
> 

-- 
Yao (éå)