Re: [PATCH] Fix compile warnings

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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
 
 

--
Crash-utility mailing list
Crash-utility@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/crash-utility

[Index of Archives]     [Fedora Development]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]

 

Powered by Linux