* [unavailable values part 1, 06/17] array element repeats, <unavailable> confused with 0.
@ 2011-02-07 14:31 Pedro Alves
2011-02-14 12:00 ` Jan Kratochvil
0 siblings, 1 reply; 9+ messages in thread
From: Pedro Alves @ 2011-02-07 14:31 UTC (permalink / raw)
To: gdb-patches
With a program that has:
struct tuple
{
int a;
int b;
};
struct tuple tarray[8];
int main()
{
memset (tarray, 0xaa, sizeof tarray);
tarray[3].a = tarray[3].b = 0;
and a tracepoint that collects:
actions
collect tarray[0].a
collect tarray[1].a
collect tarray[3].a
collect tarray[3].b
collect tarray[4].b
collect tarray[5].b
end
We'd get:
(gdb) set print repeats 1
(gdb) set print pretty on
(gdb) p /x tarray
$3 = {{
a = 0xaaaaaaaa,
b = <unavailable>
} <repeats 2 times>, {
a = <unavailable>,
b = <unavailable>
} <repeats 2 times>, {
a = <unavailable>,
b = 0xaaaaaaaa
}, {
a = <unavailable>,
b = <unavailable>
} <repeats 3 times>}
(gdb)
Note that we confuse <unavailable> with 0x0.
That is, this part is wrong:
... {
a = <unavailable>,
b = <unavailable>
} <repeats 2 times>,
as tarray[3] had been completely collected,
but it has value 0, which GDB confused with
the previous <unavailable>.
The patch below fixes this. No isolated test: I'm
covering it in a larger test that tests a bunch
of <unavailable> cases.
--
Pedro Alves
2011-02-07 Pedro Alves <pedro@codesourcery.com>
gdb/
* value.h (value_available_contents_eq): Declare.
* value.c (find_first_range_overlap): New function.
(value_available_contents_eq): New function.
* valprint.c (val_print_array_elements): Use
value_available_contents_eq.
* ada-valprint.c (val_print_packed_array_elements): Use
value_available_contents_eq.
* jv-valprint.c (java_value_print): Use
value_available_contents_eq.
---
gdb/ada-valprint.c | 4 +-
gdb/jv-valprint.c | 7 ++-
gdb/valprint.c | 8 ++--
gdb/value.c | 101 +++++++++++++++++++++++++++++++++++++++++++++++++++++
gdb/value.h | 25 +++++++++++++
5 files changed, 139 insertions(+), 6 deletions(-)
Index: src/gdb/value.h
===================================================================
--- src.orig/gdb/value.h 2011-02-07 11:15:02.926705996 +0000
+++ src/gdb/value.h 2011-02-07 11:15:23.176706001 +0000
@@ -374,6 +374,31 @@ extern int value_bytes_available (const
extern void mark_value_bytes_unavailable (struct value *value,
int offset, int length);
+/* Compare LENGTH bytes of VAL1's contents starting at OFFSET1 with
+ LENGTH bytes of VAL2's contents starting at OFFSET2. Returns true
+ iff the set of available contents match. Unavailable contents
+ compare equal with unavailable contents, and different with any
+ available byte. For example, if 'x's represent an unavailable
+ byte, and 'V' and 'Z' represent different available bytes, in a
+ value with length 16:
+
+ offset: 0 4 8 12 16
+ contents: xxxxVVVVxxxxVVZZ
+
+ then:
+
+ value_available_contents_eq(val, 0, val, 8, 6) => 1
+ value_available_contents_eq(val, 0, val, 4, 4) => 1
+ value_available_contents_eq(val, 0, val, 8, 8) => 0
+ value_available_contents_eq(val, 4, val, 12, 2) => 1
+ value_available_contents_eq(val, 4, val, 12, 4) => 0
+ value_available_contents_eq(val, 3, val, 4, 4) => 0
+*/
+
+extern int value_available_contents_eq (const struct value *val1, int offset1,
+ const struct value *val2, int offset2,
+ int length);
+
/* Read LENGTH bytes of memory starting at MEMADDR into BUFFER, which
is (or will be copied to) VAL's contents buffer offset by
EMBEDDED_OFFSET (that is, to &VAL->contents[EMBEDDED_OFFSET]).
Index: src/gdb/value.c
===================================================================
--- src.orig/gdb/value.c 2011-02-07 11:12:35.406706000 +0000
+++ src/gdb/value.c 2011-02-07 11:15:23.176706001 +0000
@@ -357,6 +357,107 @@ mark_value_bytes_unavailable (struct val
}
}
+/* Find the first range in RANGES that overlaps the range defined by
+ OFFSET and LENGTH, starting at element POS in the RANGES vector,
+ Returns the index into RANGES where such overlapping range was
+ found, or -1 if none was found. */
+
+static int
+find_first_range_overlap (VEC(range_s) *ranges, int pos,
+ int offset, int length)
+{
+ range_s *r;
+ int i;
+
+ for (i = pos; VEC_iterate (range_s, ranges, i, r); i++)
+ if (ranges_overlap (r->offset, r->length, offset, length))
+ return i;
+
+ return -1;
+}
+
+int
+value_available_contents_eq (const struct value *val1, int offset1,
+ const struct value *val2, int offset2,
+ int length)
+{
+ int org_len = length;
+ int org_offset1 = offset1;
+ int org_offset2 = offset2;
+ int idx1 = 0, idx2 = 0;
+ int prev_avail;
+
+ /* This routine is used by printing routines, where we should
+ already have read the value. Note that we only know whether a
+ value chunk is available if we've tried to read it. */
+ gdb_assert (!val1->lazy && !val2->lazy);
+
+ /* The offset from either ORG_OFFSET1 or ORG_OFFSET2 where the
+ available contents we haven't compared yet start. */
+ prev_avail = 0;
+
+ while (length > 0)
+ {
+ range_s *r1, *r2;
+ ULONGEST l1, h1;
+ ULONGEST l2, h2;
+
+ idx1 = find_first_range_overlap (val1->unavailable, idx1,
+ offset1, length);
+ idx2 = find_first_range_overlap (val2->unavailable, idx2,
+ offset2, length);
+
+ /* The usual case is for both values to be completely available. */
+ if (idx1 == -1 && idx2 == -1)
+ return (memcmp (val1->contents + org_offset1 + prev_avail,
+ val2->contents + org_offset2 + prev_avail,
+ org_len - prev_avail) == 0);
+ /* The contents only match equal if the available set matches as
+ well. */
+ else if (idx1 == -1 || idx2 == -1)
+ return 0;
+
+ gdb_assert (idx1 != -1 && idx2 != -1);
+
+ r1 = VEC_index (range_s, val1->unavailable, idx1);
+ r2 = VEC_index (range_s, val2->unavailable, idx2);
+
+ /* Get the unavailable windows intersected by the incoming
+ ranges. The first and last ranges that overlap the argument
+ range may be wider than said incoming arguments ranges. */
+ l1 = max (offset1, r1->offset);
+ h1 = min (offset1 + length, r1->offset + r1->length);
+
+ l2 = max (offset2, r2->offset);
+ h2 = min (offset2 + length, r2->offset + r2->length);
+
+ /* Make them relative to the respective start offsets, so we can
+ compare them for equality. */
+ l1 -= offset1;
+ h1 -= offset1;
+
+ l2 -= offset2;
+ h2 -= offset2;
+
+ /* Different availability, no match. */
+ if (l1 != l2 || h1 != h2)
+ return 0;
+
+ /* Compare the _available_ contents. */
+ if (memcmp (val1->contents + org_offset1 + prev_avail,
+ val2->contents + org_offset2 + prev_avail,
+ l2 - prev_avail) != 0)
+ return 0;
+
+ prev_avail += h1;
+ length -= h1;
+ offset1 += h1;
+ offset2 += h1;
+ }
+
+ return 1;
+}
+
/* Prototypes for local functions. */
static void show_values (char *, int);
Index: src/gdb/valprint.c
===================================================================
--- src.orig/gdb/valprint.c 2011-02-07 11:13:09.296706002 +0000
+++ src/gdb/valprint.c 2011-02-07 11:15:23.186706002 +0000
@@ -1242,9 +1242,11 @@ val_print_array_elements (struct type *t
rep1 = i + 1;
reps = 1;
while (rep1 < len
- && memcmp (valaddr + embedded_offset + i * eltlen,
- valaddr + embedded_offset + rep1 * eltlen,
- eltlen) == 0)
+ && value_available_contents_eq (val,
+ embedded_offset + i * eltlen,
+ val,
+ embedded_offset + rep1 * eltlen,
+ eltlen))
{
++reps;
++rep1;
Index: src/gdb/ada-valprint.c
===================================================================
--- src.orig/gdb/ada-valprint.c 2011-02-07 10:54:23.076706000 +0000
+++ src/gdb/ada-valprint.c 2011-02-07 11:15:23.186706002 +0000
@@ -200,7 +200,9 @@ val_print_packed_array_elements (struct
(i * bitsize) / HOST_CHAR_BIT,
(i * bitsize) % HOST_CHAR_BIT,
bitsize, elttype);
- if (memcmp (value_contents (v0), value_contents (v1), eltlen) != 0)
+ if (!value_available_contents_eq (v0, value_embedded_offset (v0),
+ v1, value_embedded_offset (v1),
+ eltlen))
break;
}
Index: src/gdb/jv-valprint.c
===================================================================
--- src.orig/gdb/jv-valprint.c 2011-02-07 10:54:23.086706001 +0000
+++ src/gdb/jv-valprint.c 2011-02-07 11:15:23.186706002 +0000
@@ -179,8 +179,11 @@ java_value_print (struct value *val, str
set_value_lazy (next_v, 1);
set_value_offset (next_v, value_offset (next_v)
+ TYPE_LENGTH (el_type));
- if (memcmp (value_contents (v), value_contents (next_v),
- TYPE_LENGTH (el_type)) != 0)
+ value_fetch_lazy (next_v);
+ if (!(value_available_contents_eq
+ (v, value_embedded_offset (v),
+ next_v, value_embedded_offset (next_v),
+ TYPE_LENGTH (el_type))))
break;
}
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [unavailable values part 1, 06/17] array element repeats, <unavailable> confused with 0.
2011-02-07 14:31 [unavailable values part 1, 06/17] array element repeats, <unavailable> confused with 0 Pedro Alves
@ 2011-02-14 12:00 ` Jan Kratochvil
2011-02-15 18:34 ` Pedro Alves
0 siblings, 1 reply; 9+ messages in thread
From: Jan Kratochvil @ 2011-02-14 12:00 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
> --- src.orig/gdb/value.h 2011-02-07 11:15:02.926705996 +0000
> +++ src/gdb/value.h 2011-02-07 11:15:23.176706001 +0000
> @@ -374,6 +374,31 @@ extern int value_bytes_available (const
> extern void mark_value_bytes_unavailable (struct value *value,
> int offset, int length);
>
> +/* Compare LENGTH bytes of VAL1's contents starting at OFFSET1 with
> + LENGTH bytes of VAL2's contents starting at OFFSET2. Returns true
/* OFFSET1 and OFFSET2 should include possible EMBEDDED_OFFSET. */
> + iff the set of available contents match. Unavailable contents
> + compare equal with unavailable contents, and different with any
> + available byte. For example, if 'x's represent an unavailable
> + byte, and 'V' and 'Z' represent different available bytes, in a
> + value with length 16:
> +
> + offset: 0 4 8 12 16
> + contents: xxxxVVVVxxxxVVZZ
> +
> + then:
> +
> + value_available_contents_eq(val, 0, val, 8, 6) => 1
> + value_available_contents_eq(val, 0, val, 4, 4) => 1
> + value_available_contents_eq(val, 0, val, 8, 8) => 0
> + value_available_contents_eq(val, 4, val, 12, 2) => 1
> + value_available_contents_eq(val, 4, val, 12, 4) => 0
> + value_available_contents_eq(val, 3, val, 4, 4) => 0
> +*/
> +
> +extern int value_available_contents_eq (const struct value *val1, int offset1,
> + const struct value *val2, int offset2,
> + int length);
> +/* Find the first range in RANGES that overlaps the range defined by
> + OFFSET and LENGTH, starting at element POS in the RANGES vector,
> + Returns the index into RANGES where such overlapping range was
> + found, or -1 if none was found. */
> +
> +static int
> +find_first_range_overlap (VEC(range_s) *ranges, int pos,
> + int offset, int length)
> +{
> + range_s *r;
> + int i;
> +
> + for (i = pos; VEC_iterate (range_s, ranges, i, r); i++)
> + if (ranges_overlap (r->offset, r->length, offset, length))
> + return i;
> +
> + return -1;
> +}
> +
> +int
> +value_available_contents_eq (const struct value *val1, int offset1,
> + const struct value *val2, int offset2,
> + int length)
> +{
> + int org_len = length;
> + int org_offset1 = offset1;
> + int org_offset2 = offset2;
All the org_* fields can be dropped by reusing other variables making the code
more readable.
> + int idx1 = 0, idx2 = 0;
> + int prev_avail;
> +
> + /* This routine is used by printing routines, where we should
> + already have read the value. Note that we only know whether a
> + value chunk is available if we've tried to read it. */
> + gdb_assert (!val1->lazy && !val2->lazy);
> +
> + /* The offset from either ORG_OFFSET1 or ORG_OFFSET2 where the
> + available contents we haven't compared yet start. */
> + prev_avail = 0;
> +
> + while (length > 0)
> + {
> + range_s *r1, *r2;
> + ULONGEST l1, h1;
> + ULONGEST l2, h2;
> +
> + idx1 = find_first_range_overlap (val1->unavailable, idx1,
> + offset1, length);
> + idx2 = find_first_range_overlap (val2->unavailable, idx2,
> + offset2, length);
> +
> + /* The usual case is for both values to be completely available. */
> + if (idx1 == -1 && idx2 == -1)
> + return (memcmp (val1->contents + org_offset1 + prev_avail,
`org_offset1 + prev_avail' -> `offset1'.
> + val2->contents + org_offset2 + prev_avail,
> + org_len - prev_avail) == 0);
`org_len - prev_avail' -> `length' to drop `org_len'.
> + /* The contents only match equal if the available set matches as
> + well. */
> + else if (idx1 == -1 || idx2 == -1)
> + return 0;
> +
> + gdb_assert (idx1 != -1 && idx2 != -1);
> +
> + r1 = VEC_index (range_s, val1->unavailable, idx1);
> + r2 = VEC_index (range_s, val2->unavailable, idx2);
> +
> + /* Get the unavailable windows intersected by the incoming
> + ranges. The first and last ranges that overlap the argument
> + range may be wider than said incoming arguments ranges. */
> + l1 = max (offset1, r1->offset);
> + h1 = min (offset1 + length, r1->offset + r1->length);
> +
> + l2 = max (offset2, r2->offset);
> + h2 = min (offset2 + length, r2->offset + r2->length);
> +
> + /* Make them relative to the respective start offsets, so we can
> + compare them for equality. */
> + l1 -= offset1;
> + h1 -= offset1;
> +
> + l2 -= offset2;
> + h2 -= offset2;
> +
> + /* Different availability, no match. */
> + if (l1 != l2 || h1 != h2)
> + return 0;
> +
> + /* Compare the _available_ contents. */
> + if (memcmp (val1->contents + org_offset1 + prev_avail,
> + val2->contents + org_offset2 + prev_avail,
> + l2 - prev_avail) != 0)
`l2 - prev_avail' is not right. `l2' is already this available chunk size,
`prev_avail' can be much larger covering all the preceding ranges,
`prev_avail' is already subtracted from `l2' by `offset1'/`offset2' above.
> + return 0;
> +
> + prev_avail += h1;
> + length -= h1;
> + offset1 += h1;
> + offset2 += h1;
> + }
> +
> + return 1;
> +}
Thanks,
Jan
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [unavailable values part 1, 06/17] array element repeats, <unavailable> confused with 0.
2011-02-14 12:00 ` Jan Kratochvil
@ 2011-02-15 18:34 ` Pedro Alves
2011-02-16 9:49 ` Jan Kratochvil
0 siblings, 1 reply; 9+ messages in thread
From: Pedro Alves @ 2011-02-15 18:34 UTC (permalink / raw)
To: gdb-patches; +Cc: Jan Kratochvil
On Monday 14 February 2011 12:00:16, Jan Kratochvil wrote:
> > --- src.orig/gdb/value.h 2011-02-07 11:15:02.926705996 +0000
> > +++ src/gdb/value.h 2011-02-07 11:15:23.176706001 +0000
> > @@ -374,6 +374,31 @@ extern int value_bytes_available (const
> > extern void mark_value_bytes_unavailable (struct value *value,
> > int offset, int length);
> >
> > +/* Compare LENGTH bytes of VAL1's contents starting at OFFSET1 with
> > + LENGTH bytes of VAL2's contents starting at OFFSET2. Returns true
>
> /* OFFSET1 and OFFSET2 should include possible EMBEDDED_OFFSET. */
Ah, I can see how someone not looking at the implementation might
not realize which "contents" are we talking about. I've extended
the describing comment.
> > +int
> > +value_available_contents_eq (const struct value *val1, int offset1,
> > + const struct value *val2, int offset2,
> > + int length)
> > +{
> > + int org_len = length;
> > + int org_offset1 = offset1;
> > + int org_offset2 = offset2;
>
> All the org_* fields can be dropped by reusing other variables making the code
> more readable.
You're right! I like that change.
> > +
> > + /* Compare the _available_ contents. */
> > + if (memcmp (val1->contents + org_offset1 + prev_avail,
> > + val2->contents + org_offset2 + prev_avail,
> > + l2 - prev_avail) != 0)
>
> `l2 - prev_avail' is not right. `l2' is already this available chunk size,
> `prev_avail' can be much larger covering all the preceding ranges,
> `prev_avail' is already subtracted from `l2' by `offset1'/`offset2' above.
Indeed. Could you double-check the patch below?
Thanks!
Pedro Alves
2011-02-15 Pedro Alves <pedro@codesourcery.com>
Jan Kratochvil <jan.kratochvil@redhat.com>
gdb/
* value.c (value_available_contents_eq): Remove redundant local
variables. Fix available contents comparision.
* value.h (value_available_contents_eq): Extend describing
comment.
---
gdb/value.c | 19 ++++---------------
gdb/value.h | 21 +++++++++++++++------
2 files changed, 19 insertions(+), 21 deletions(-)
Index: src/gdb/value.c
===================================================================
--- src.orig/gdb/value.c 2011-02-15 16:19:15.000000000 +0000
+++ src/gdb/value.c 2011-02-15 17:51:58.018123001 +0000
@@ -533,21 +533,13 @@ value_available_contents_eq (const struc
const struct value *val2, int offset2,
int length)
{
- int org_len = length;
- int org_offset1 = offset1;
- int org_offset2 = offset2;
int idx1 = 0, idx2 = 0;
- int prev_avail;
/* This routine is used by printing routines, where we should
already have read the value. Note that we only know whether a
value chunk is available if we've tried to read it. */
gdb_assert (!val1->lazy && !val2->lazy);
- /* The offset from either ORG_OFFSET1 or ORG_OFFSET2 where the
- available contents we haven't compared yet start. */
- prev_avail = 0;
-
while (length > 0)
{
range_s *r1, *r2;
@@ -561,9 +553,9 @@ value_available_contents_eq (const struc
/* The usual case is for both values to be completely available. */
if (idx1 == -1 && idx2 == -1)
- return (memcmp (val1->contents + org_offset1 + prev_avail,
- val2->contents + org_offset2 + prev_avail,
- org_len - prev_avail) == 0);
+ return (memcmp (val1->contents + offset1,
+ val2->contents + offset2,
+ length) == 0);
/* The contents only match equal if the available set matches as
well. */
else if (idx1 == -1 || idx2 == -1)
@@ -596,12 +588,9 @@ value_available_contents_eq (const struc
return 0;
/* Compare the _available_ contents. */
- if (memcmp (val1->contents + org_offset1 + prev_avail,
- val2->contents + org_offset2 + prev_avail,
- l2 - prev_avail) != 0)
+ if (memcmp (val1->contents + offset1, val2->contents + offset2, l1) != 0)
return 0;
- prev_avail += h1;
length -= h1;
offset1 += h1;
offset2 += h1;
Index: src/gdb/value.h
===================================================================
--- src.orig/gdb/value.h 2011-02-14 21:51:17.000000000 +0000
+++ src/gdb/value.h 2011-02-15 18:11:23.938123001 +0000
@@ -379,12 +379,21 @@ extern void mark_value_bytes_unavailable
int offset, int length);
/* Compare LENGTH bytes of VAL1's contents starting at OFFSET1 with
- LENGTH bytes of VAL2's contents starting at OFFSET2. Returns true
- iff the set of available contents match. Unavailable contents
- compare equal with unavailable contents, and different with any
- available byte. For example, if 'x's represent an unavailable
- byte, and 'V' and 'Z' represent different available bytes, in a
- value with length 16:
+ LENGTH bytes of VAL2's contents starting at OFFSET2.
+
+ Note that "contents" refers to the whole value's contents
+ (value_contents_all), without any embedded offset adjustment. For
+ example, to compare a complete object value with itself, including
+ its enclosing type chunk, you'd do:
+
+ int len = TYPE_LENGTH (check_typedef (value_enclosing_type (val)));
+ value_available_contents (val, 0, val, 0, len);
+
+ Returns true iff the set of available contents match. Unavailable
+ contents compare equal with unavailable contents, and different
+ with any available byte. For example, if 'x's represent an
+ unavailable byte, and 'V' and 'Z' represent different available
+ bytes, in a value with length 16:
offset: 0 4 8 12 16
contents: xxxxVVVVxxxxVVZZ
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [unavailable values part 1, 06/17] array element repeats, <unavailable> confused with 0.
2011-02-15 18:34 ` Pedro Alves
@ 2011-02-16 9:49 ` Jan Kratochvil
2011-02-16 10:23 ` Pedro Alves
0 siblings, 1 reply; 9+ messages in thread
From: Jan Kratochvil @ 2011-02-16 9:49 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
On Tue, 15 Feb 2011 19:28:35 +0100, Pedro Alves wrote:
> Indeed. Could you double-check the patch below?
It seems correct to me now.
> - if (memcmp (val1->contents + org_offset1 + prev_avail,
> - val2->contents + org_offset2 + prev_avail,
> - l2 - prev_avail) != 0)
> + if (memcmp (val1->contents + offset1, val2->contents + offset2, l1) != 0)
[nitpick] GNU Coding standards says 78 columns (this is 79), OTOH Joel said he
already uses 80. http://sourceware.org/ml/gdb-patches/2011-01/msg00255.html
Thanks,
Jan
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [unavailable values part 1, 06/17] array element repeats, <unavailable> confused with 0.
2011-02-16 9:49 ` Jan Kratochvil
@ 2011-02-16 10:23 ` Pedro Alves
2011-02-16 11:12 ` Jan Kratochvil
0 siblings, 1 reply; 9+ messages in thread
From: Pedro Alves @ 2011-02-16 10:23 UTC (permalink / raw)
To: Jan Kratochvil; +Cc: gdb-patches
On Wednesday 16 February 2011 09:28:50, Jan Kratochvil wrote:
> On Tue, 15 Feb 2011 19:28:35 +0100, Pedro Alves wrote:
> > Indeed. Could you double-check the patch below?
>
> It seems correct to me now.
Thanks for checking! I've applied the patch now.
> > - if (memcmp (val1->contents + org_offset1 + prev_avail,
> > - val2->contents + org_offset2 + prev_avail,
> > - l2 - prev_avail) != 0)
> > + if (memcmp (val1->contents + offset1, val2->contents + offset2, l1) != 0)
>
> [nitpick] GNU Coding standards says 78 columns (this is 79),
Hmm, it does? Can you point it out, please? I'm not finding it.
I usually follow the "whatever emacs does/wants is what the
coding standards say" pseudo-rule.
> OTOH Joel said he already uses 80.
> http://sourceware.org/ml/gdb-patches/2011-01/msg00255.html
I'm also using 80, except on comments, docs and changelogs, where
I just do M-q (fill-paragraph) and let emacs decide automatically.
And I tend to flip whitespace-mode on often to easily check/see
exceeding lines and spurious whitespace:
<http://www.emacswiki.org/emacs/WhiteSpace>
Which I clearly remember doing to check that line wasn't
exceeding. :-)
Anyway, I just went ahead and wrapped the line before
committing.
OOC, I found this emacs wiki page:
<http://www.emacswiki.org/emacs/EightyColumnRule>
--
Pedro Alves
2011-02-16 Pedro Alves <pedro@codesourcery.com>
Jan Kratochvil <jan.kratochvil@redhat.com>
gdb/
* value.c (value_available_contents_eq): Remove redundant local
variables. Fix available contents comparision.
* value.h (value_available_contents_eq): Extend describing
comment.
---
gdb/value.c | 21 ++++++---------------
gdb/value.h | 21 +++++++++++++++------
2 files changed, 21 insertions(+), 21 deletions(-)
Index: src/gdb/value.c
===================================================================
--- src.orig/gdb/value.c 2011-02-15 16:19:15.000000000 +0000
+++ src/gdb/value.c 2011-02-16 09:52:24.906002009 +0000
@@ -533,21 +533,13 @@ value_available_contents_eq (const struc
const struct value *val2, int offset2,
int length)
{
- int org_len = length;
- int org_offset1 = offset1;
- int org_offset2 = offset2;
int idx1 = 0, idx2 = 0;
- int prev_avail;
/* This routine is used by printing routines, where we should
already have read the value. Note that we only know whether a
value chunk is available if we've tried to read it. */
gdb_assert (!val1->lazy && !val2->lazy);
- /* The offset from either ORG_OFFSET1 or ORG_OFFSET2 where the
- available contents we haven't compared yet start. */
- prev_avail = 0;
-
while (length > 0)
{
range_s *r1, *r2;
@@ -561,9 +553,9 @@ value_available_contents_eq (const struc
/* The usual case is for both values to be completely available. */
if (idx1 == -1 && idx2 == -1)
- return (memcmp (val1->contents + org_offset1 + prev_avail,
- val2->contents + org_offset2 + prev_avail,
- org_len - prev_avail) == 0);
+ return (memcmp (val1->contents + offset1,
+ val2->contents + offset2,
+ length) == 0);
/* The contents only match equal if the available set matches as
well. */
else if (idx1 == -1 || idx2 == -1)
@@ -596,12 +588,11 @@ value_available_contents_eq (const struc
return 0;
/* Compare the _available_ contents. */
- if (memcmp (val1->contents + org_offset1 + prev_avail,
- val2->contents + org_offset2 + prev_avail,
- l2 - prev_avail) != 0)
+ if (memcmp (val1->contents + offset1,
+ val2->contents + offset2,
+ l1) != 0)
return 0;
- prev_avail += h1;
length -= h1;
offset1 += h1;
offset2 += h1;
Index: src/gdb/value.h
===================================================================
--- src.orig/gdb/value.h 2011-02-14 21:51:17.000000000 +0000
+++ src/gdb/value.h 2011-02-15 18:11:23.938123001 +0000
@@ -379,12 +379,21 @@ extern void mark_value_bytes_unavailable
int offset, int length);
/* Compare LENGTH bytes of VAL1's contents starting at OFFSET1 with
- LENGTH bytes of VAL2's contents starting at OFFSET2. Returns true
- iff the set of available contents match. Unavailable contents
- compare equal with unavailable contents, and different with any
- available byte. For example, if 'x's represent an unavailable
- byte, and 'V' and 'Z' represent different available bytes, in a
- value with length 16:
+ LENGTH bytes of VAL2's contents starting at OFFSET2.
+
+ Note that "contents" refers to the whole value's contents
+ (value_contents_all), without any embedded offset adjustment. For
+ example, to compare a complete object value with itself, including
+ its enclosing type chunk, you'd do:
+
+ int len = TYPE_LENGTH (check_typedef (value_enclosing_type (val)));
+ value_available_contents (val, 0, val, 0, len);
+
+ Returns true iff the set of available contents match. Unavailable
+ contents compare equal with unavailable contents, and different
+ with any available byte. For example, if 'x's represent an
+ unavailable byte, and 'V' and 'Z' represent different available
+ bytes, in a value with length 16:
offset: 0 4 8 12 16
contents: xxxxVVVVxxxxVVZZ
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [unavailable values part 1, 06/17] array element repeats, <unavailable> confused with 0.
2011-02-16 10:23 ` Pedro Alves
@ 2011-02-16 11:12 ` Jan Kratochvil
2011-02-16 18:26 ` Michael Snyder
0 siblings, 1 reply; 9+ messages in thread
From: Jan Kratochvil @ 2011-02-16 11:12 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
On Wed, 16 Feb 2011 11:15:19 +0100, Pedro Alves wrote:
> On Wednesday 16 February 2011 09:28:50, Jan Kratochvil wrote:
> > [nitpick] GNU Coding standards says 78 columns (this is 79),
>
> Hmm, it does? Can you point it out, please? I'm not finding it.
Not explicitly but the options line for GNU indent
http://www.gnu.org/prep/standards/standards.html#Formatting
has no -lc option and GNU indent defaults to 78 columns.
> <http://www.emacswiki.org/emacs/EightyColumnRule>
OK, it seems 80 columns is OK.
Thanks,
Jan
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [unavailable values part 1, 06/17] array element repeats, <unavailable> confused with 0.
2011-02-16 11:12 ` Jan Kratochvil
@ 2011-02-16 18:26 ` Michael Snyder
2011-02-16 19:46 ` Eli Zaretskii
0 siblings, 1 reply; 9+ messages in thread
From: Michael Snyder @ 2011-02-16 18:26 UTC (permalink / raw)
To: Jan Kratochvil; +Cc: Pedro Alves, gdb-patches
Jan Kratochvil wrote:
> On Wed, 16 Feb 2011 11:15:19 +0100, Pedro Alves wrote:
>> On Wednesday 16 February 2011 09:28:50, Jan Kratochvil wrote:
>>> [nitpick] GNU Coding standards says 78 columns (this is 79),
>> Hmm, it does? Can you point it out, please? I'm not finding it.
>
> Not explicitly but the options line for GNU indent
> http://www.gnu.org/prep/standards/standards.html#Formatting
>
> has no -lc option and GNU indent defaults to 78 columns.
>
>
>> <http://www.emacswiki.org/emacs/EightyColumnRule>
>
> OK, it seems 80 columns is OK.
No way. Emacs wraps at 79, so 80 is definitely *not* ok by me...
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [unavailable values part 1, 06/17] array element repeats, <unavailable> confused with 0.
2011-02-16 18:26 ` Michael Snyder
@ 2011-02-16 19:46 ` Eli Zaretskii
2011-02-16 19:55 ` Michael Snyder
0 siblings, 1 reply; 9+ messages in thread
From: Eli Zaretskii @ 2011-02-16 19:46 UTC (permalink / raw)
To: Michael Snyder; +Cc: jan.kratochvil, pedro, gdb-patches
> Date: Wed, 16 Feb 2011 10:17:44 -0800
> From: Michael Snyder <msnyder@vmware.com>
> CC: Pedro Alves <pedro@codesourcery.com>, "gdb-patches@sourceware.org" <gdb-patches@sourceware.org>
>
> >> <http://www.emacswiki.org/emacs/EightyColumnRule>
> >
> > OK, it seems 80 columns is OK.
>
> No way. Emacs wraps at 79, so 80 is definitely *not* ok by me...
Actually, Emacs wraps at 79 only on text-mode terminals these days.
In a GUI session, it wraps at 80.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [unavailable values part 1, 06/17] array element repeats, <unavailable> confused with 0.
2011-02-16 19:46 ` Eli Zaretskii
@ 2011-02-16 19:55 ` Michael Snyder
0 siblings, 0 replies; 9+ messages in thread
From: Michael Snyder @ 2011-02-16 19:55 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: jan.kratochvil, pedro, gdb-patches
Eli Zaretskii wrote:
>> Date: Wed, 16 Feb 2011 10:17:44 -0800
>> From: Michael Snyder <msnyder@vmware.com>
>> CC: Pedro Alves <pedro@codesourcery.com>, "gdb-patches@sourceware.org" <gdb-patches@sourceware.org>
>>
>>>> <http://www.emacswiki.org/emacs/EightyColumnRule>
>>> OK, it seems 80 columns is OK.
>> No way. Emacs wraps at 79, so 80 is definitely *not* ok by me...
>
> Actually, Emacs wraps at 79 only on text-mode terminals these days.
> In a GUI session, it wraps at 80.
Good to know. I use a text-mode terminal.
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2011-02-16 19:47 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-02-07 14:31 [unavailable values part 1, 06/17] array element repeats, <unavailable> confused with 0 Pedro Alves
2011-02-14 12:00 ` Jan Kratochvil
2011-02-15 18:34 ` Pedro Alves
2011-02-16 9:49 ` Jan Kratochvil
2011-02-16 10:23 ` Pedro Alves
2011-02-16 11:12 ` Jan Kratochvil
2011-02-16 18:26 ` Michael Snyder
2011-02-16 19:46 ` Eli Zaretskii
2011-02-16 19:55 ` Michael Snyder
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox