This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [unavailable values part 1, 01/17] base support for unavailable value contents
On Thu, 10 Feb 2011 20:30:26 +0100, Pedro Alves wrote:
> +/* Defines an [OFFSET, OFFSET + LENGTH) range. */
> +
> +struct range
> +{
> + /* Lowest offset in the range. */
> + int offset;
> +
> + /* Length of the range. */
> + int length;
> +};
I would find [LOW, HIGH) fields more readable as the code now still calculates
offset + length back and forth all over the code. FYI, not trying to require
it, though.
> +/* Returns true if RANGES contains any range that overlaps [OFFSET,
> + OFFSET+LENGTH). */
> +
> +static int
> +ranges_contain_p (VEC(range_s) *ranges, int offset, int length)
Couldn't even this function stick with the `overlap' term?
`contain' associates to me:
Returns true if each byte of [OFFSET, OFFSET+LENGTH) is overlapping with any
range in the RANGES list.
(My English association may not be right, though.)
> @@ -206,6 +310,11 @@ struct value
> + /* Unavailable ranges in CONTENTS. We mark unavailable ranges,
> + rather than available, since the common and default case is for a
> + value to be available. This is filled in at value read time. */
> + VEC(range_s) *unavailable;
Was there considered the opposite way to have a list of available ranges?
Besides cleaning up the inversion code implemented by this patchset in
read_value_memory it would also enable storing discontiguous memory with
a value.
Currently if you store inferior C++ object into a $convenience_variable you
cannot do much with it as it can no longer read the virtual method table
- besides it may no longer exist in the inferior the current code will not
even try to read it from the inferior.
I faced it also with archer-jankratochvil-vla - if you have a very large
inferior array printing only some slices/subsets of it - you still have to
store for $convenience_variable the whole range between first byte and last
byte accessed, despite most of the ranges in between get never accessed. You
will mostly print some slices/subsets because the whole array is too large.
So I was considering to turn value->contents into some discontiguous ranges
and this patch could also benefit from it.
> };
>
> +int
> +value_bytes_available (const struct value *value, int offset, int length)
ctags will never find a comment defined in a .h file. I would prefer at least
a stub comment:
/* Function comment present at the declaration. */
Many legacy functions just do not have any comment so one just does not search
more for a comment when there isn't any shown on the ctags-jump.
> + i = VEC_lower_bound (range_s, value->unavailable, &newr, range_lessthan);
> + if (i > 0)
> + {
> + struct range *bef = VEC_index (range_s, value->unavailable, i - i);
While this patch revisiou fixed two bugs
it has introduced a new bug - "i - i" -> "i - 1".
Thanks,
Jan