<!doctype html public "-//w3c//dtd html 4.0 transitional//en">
<html>
Bernhard Walle wrote:
<blockquote TYPE=CITE>Hello,
<p>patch attached. Please consider to add the changes mainline. But
<br>please also check back all changes.
<p>Some comments:
<p> gdb/dwarf2-frame.c:
<br> - I think the buf += overwrites the ++, at least
my tests with
<br> some test codes showed that.
<p> tools.c:
<br> - this is really strange, if index = 0, then the
assignment
<br> doesn't make sense. If it's random,
it also doesn't make sense. :)
<br> Didn't have time to dig into the whole
logic of this hashtable.
<p>Regards,
<br> Bernhard
<p> </blockquote>
<tt></tt>
<p><br><tt>Hi Bernhard,</tt><tt></tt>
<p><tt>Thanks -- comments on each patch below...</tt><tt></tt>
<p><tt>--------------------------------</tt><tt></tt>
<p><tt>gdb-6.1.patch:</tt><tt></tt>
<p><tt>
else if (*augmentation == 'P')</tt>
<br><tt>
{</tt>
<br><tt>
/* Skip. */</tt>
<br><tt>-
buf += size_of_encoded_value (*buf++);</tt>
<br><tt>+
buf += size_of_encoded_value (*buf);</tt>
<br><tt>
augmentation++;</tt>
<br><tt>
}</tt><tt></tt>
<p><tt>This patch makes me a litte nervous. I see that gdb 6.5 does
this:</tt><tt></tt>
<p><tt> else if (*augmentation
== 'P')</tt>
<br><tt>
{</tt>
<br><tt>
/* Skip. Avoid indirection since we throw away the result.
*/</tt>
<br><tt>
gdb_byte encoding = (*buf++) & ~DW_EH_PE_indirect;</tt>
<br><tt>
read_encoded_value (unit, encoding, buf, &bytes_read);</tt>
<br><tt>
buf += bytes_read;</tt>
<br><tt>
augmentation++;</tt>
<br><tt>
}</tt><tt></tt>
<p><tt>Are they equivalent?</tt><tt></tt>
<p><tt>--------------------------------</tt><tt></tt>
<p><tt>lkcd_common.c:</tt><tt></tt>
<p><tt> int</tt>
<br><tt> lkcd_lseek(physaddr_t paddr)</tt>
<br><tt> {</tt>
<br><tt>- long i;</tt>
<br><tt>+ long i = 0;</tt>
<br><tt> int err;</tt>
<br><tt> int eof;</tt>
<br><tt> void *dp;</tt><tt></tt>
<p><tt>Although it looks like it really doesn't matter where "i" starts,</tt>
<br><tt>this is fine.</tt><tt></tt>
<p><tt>--------------------------------</tt>
<br><tt> </tt>
<br><tt> symbols.c:</tt><tt></tt>
<p><tt> ulong start, end;</tt>
<br><tt> char *modbuf;</tt>
<br><tt> ulong maxchunk, alloc;</tt>
<br><tt>- long offset;</tt>
<br><tt>+ long offset = 0;</tt><tt></tt>
<p><tt>That's fine...</tt><tt></tt>
<p><tt>--------------------------------</tt><tt></tt>
<p><tt>task.c:</tt><tt></tt>
<p><tt> void</tt>
<br><tt> dump_task_table(int verbose)</tt>
<br><tt> {</tt>
<br><tt>- int i, nr_cpus;</tt>
<br><tt>+ int i, nr_cpus = 1;</tt>
<br><tt> struct task_context
*tc;</tt>
<br><tt> char buf[BUFSIZE];</tt>
<br><tt> int others, wrap, flen;</tt><tt></tt>
<p><tt>Actually that's a bug -- the subsequent assignment should read:</tt><tt></tt>
<p><tt> nr_cpus = kt->kernel_NR_CPUS ? kt->kernel_NR_CPUS : NR_CPUS;</tt><tt></tt>
<p><tt>--------------------------------</tt><tt></tt>
<p><tt>tools.c:</tt><tt></tt>
<p><tt> eval_common(char *s, int flags, int *errptr, struct number_option
*np)</tt>
<br><tt> {</tt>
<br><tt> char *p1, *p2;</tt>
<br><tt>- char *op, opcode;</tt>
<br><tt>+ char *op, opcode = 0;</tt>
<br><tt> ulong value1;</tt>
<br><tt> ulong value2;</tt>
<br><tt> ulonglong ll_value1;</tt><tt></tt>
<p><tt>That's good...</tt><tt></tt>
<p><tt>@@ -3672,7 +3672,7 @@ hq_entry_exists(ulong value)</tt>
<br><tt> struct hash_table *ht;</tt>
<br><tt> struct hq_entry *list_entry;</tt>
<br><tt> long hqi;</tt>
<br><tt>- long index;</tt>
<br><tt>+ long index = 0;</tt><tt></tt>
<p><tt>Good point on this one. That function is never called by anybody,</tt>
<br><tt>but was added upon request of an extension writer. It's just</tt>
<br><tt>walking the entries in a hash list looking for a matching value</tt>
<br><tt>in an entry, and shouldn't be writing anything into an entry (and</tt>
<br><tt>certainly not an uninitialized anything!). In any case, there
is</tt>
<br><tt>no need for the index variable to exist at all, so I'll just rip</tt>
<br><tt>it out.</tt><tt></tt>
<p><tt>Thanks,</tt>
<br><tt> Dave</tt>
<br><tt></tt>
<br><tt></tt> </html>