Re: bitpos expansion patches summary

Siddhesh Poyarekar
2012-08-10T01:44:24Z

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