[Crash-utility] [PATCH] Fix compile warnings

Dave Anderson anderson at redhat.com
Wed Jan 17 14:48:28 UTC 2007


Bernhard Walle wrote:

> Hello,
>
> patch attached. Please consider to add the changes mainline. But
> please also check back all changes.
>
> Some comments:
>
>   gdb/dwarf2-frame.c:
>     - I think the buf += overwrites the ++, at least my tests with
>       some test codes showed that.
>
>   tools.c:
>     - this is really strange, if index = 0, then the assignment
>       doesn't make sense. If it's random, it also doesn't make sense.  :)
>       Didn't have time to dig into the whole logic of this hashtable.
>
> Regards,
>  Bernhard
>
>

Hi Bernhard,

Thanks -- comments on each patch below...

--------------------------------

gdb-6.1.patch:

            else if (*augmentation == 'P')
            {
              /* Skip.  */
-             buf += size_of_encoded_value (*buf++);
+             buf += size_of_encoded_value (*buf);
              augmentation++;
            }

This patch makes me a litte nervous.  I see that gdb 6.5 does this:

          else if (*augmentation == 'P')
            {
              /* Skip.  Avoid indirection since we throw away the result.  */
              gdb_byte encoding = (*buf++) & ~DW_EH_PE_indirect;
              read_encoded_value (unit, encoding, buf, &bytes_read);
              buf += bytes_read;
              augmentation++;
            }

Are they equivalent?

--------------------------------

lkcd_common.c:

 int
 lkcd_lseek(physaddr_t paddr)
 {
-        long i;
+        long i = 0;
         int err;
         int eof;
         void *dp;

Although it looks like it really doesn't matter where "i" starts,
this is fine.

--------------------------------

  symbols.c:

        ulong start, end;
        char *modbuf;
        ulong maxchunk, alloc;
-       long offset;
+       long offset = 0;

That's fine...

--------------------------------

task.c:

 void
 dump_task_table(int verbose)
 {
-       int i, nr_cpus;
+       int i, nr_cpus = 1;
        struct task_context *tc;
        char buf[BUFSIZE];
        int others, wrap, flen;

Actually that's a bug -- the subsequent assignment should read:

  nr_cpus = kt->kernel_NR_CPUS ? kt->kernel_NR_CPUS : NR_CPUS;

--------------------------------

tools.c:

 eval_common(char *s, int flags, int *errptr, struct number_option *np)
 {
        char *p1, *p2;
-       char *op, opcode;
+       char *op, opcode = 0;
        ulong value1;
        ulong value2;
        ulonglong ll_value1;

That's good...

@@ -3672,7 +3672,7 @@ hq_entry_exists(ulong value)
        struct hash_table *ht;
        struct hq_entry *list_entry;
        long hqi;
-       long index;
+       long index = 0;

Good point on this one.  That function is never called by anybody,
but was added upon request of an extension writer.  It's just
walking the entries in a hash list looking for a matching value
in an entry, and shouldn't be writing anything into an entry (and
certainly not an uninitialized anything!).  In any case, there is
no need for the index variable to exist at all, so I'll just rip
it out.

Thanks,
  Dave


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/crash-utility/attachments/20070117/c43f710a/attachment.htm>


More information about the Crash-utility mailing list