Re: bitpos expansion patches summary

[Jan Kratochvil <jan.kratochvil@redhat.com>]

Hi Siddhesh,

just some incremental notes, still looking at it.


Thanks,
Jan


 @@ -1406,6 +1406,15 @@ lookup_struct_elt_type (struct type *type, char *name, int noerr)
    error (_("Type %s has no component named %s."), typename, name);
  }
  
 +/* Ensure that the input TYPE is not larger than the maximum capacity of the
 +   host system, i.e. size_t.  */
 +void
 +ensure_type_fits_sizet (const struct type *type)
+
+Make it ensure_type_fits_size_t.  And there should be 'error' in its name.
+For example 'type_fits_size_t_or_error' or something like that.
+
+
 +{
 +  if (!TYPE_LENGTH_FITS_SIZET (type))
 +    error (_("Target object too large for host GDB memory."));
+
+Please print the sizes.  Also this message is present at multiple places so
+maybe rather make a function for unconditionally printing the error?
+
+
 +}
 +
  /* Lookup the vptr basetype/fieldno values for TYPE.
     If found store vptr_basetype in *BASETYPEP if non-NULL, and return
     vptr_fieldno.  Also, if found and basetype is from the same objfile,
 @@ -1059,6 +1059,10 @@ extern void allocate_gnat_aux_type (struct type *);
     so you only have to call check_typedef once.  Since allocate_value
     calls check_typedef, TYPE_LENGTH (VALUE_TYPE (X)) is safe.  */
  #define TYPE_LENGTH(thistype) (thistype)->length
+
+Empty line.
+
 +/* Make sure that TYPE_LENGTH fits into a size_t.  */
 +#define TYPE_LENGTH_FITS_SIZET(thistype) ((size_t) TYPE_LENGTH (thistype) \
 +					  == TYPE_LENGTH (thistype))
+
+Make it TYPE_LENGTH_FITS_SIZE_T, please.  And I think this macro is not needed,
+inline it.  (It does not access internal fields of the type structures.)
+
+And (a) check it already for ssize_t.  Because the code is not safe enough to
+properly handle unsigned sizes everywhere without overflows.  (b) Make there
+some reserve, anything close to ssize_t will never get successfully xmalloc-ed
+anyway.  Some maximum size could be: ((size_t) -1) / 4.  Depending on SSIZE_MAX
+may not be compatible enough I guess.
+
+
 +
  /* Note that TYPE_CODE can be TYPE_CODE_TYPEDEF, so if you want the real
     type, you need to do TYPE_CODE (check_type (this_type)).  */
  #define TYPE_CODE(thistype) TYPE_MAIN_TYPE(thistype)->code