Re: bitpos expansion patches summary

[Siddhesh Poyarekar <siddhesh@redhat.com>]

On Thu, 9 Aug 2012 22:03:56 +0200, Jan wrote:
>  +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.

OK, will make this type_fits_size_t_or_error.

>  +{
>  +  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? +

Could you please give an example of this? I didn't think that there are
any such checks in the source yet.

>  +/* 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.) +

I had kept it for possible future need if someone wants to only check
if a type fits in and not throw an error. I will inline it.

> +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. +

I had thought of that, but I figured that instead of guessing a value,
I would be better off only doing the size_t check (i.e. code sanity) and
leave the question of whether a type gets successfully malloc'd or not
to the OS.

For ssize_t, I could add an extra boolean argument to
type_fits_size_t_or_error that indicates whether the type is signed or
not.

Regards,
Siddhesh