Fix compile warnings

Bernhard Walle wrote:

> Hello,
>
> * Dave Anderson <andersonredhat.com> [2007-01-17
15:48]:
> >
> > 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?
>
> Of course not.  After
thinking a bit I think the correct fix would
> be:
>
>         buf += size_of_encoded_value(*buf);
>         buf++;
>
> Do you aggree?
>

I can't say with any confidence.  I was trying to correlate
the difference
between the way it's done in gdb-6.5 vs 6.1, and got lost
doing so...

Just curious -- how do you manage to get crash to run
through that
code path?  (I put an exit() there, but can't seem to get
there...)


> Attached I have some more fixes for the GDB patch (so
this is no patch
> for crash but should be added to gdb-6.1.patch). I saw
that there's
> some internal memset() with different paramter order in
GDB [1], but
> that's not taken in the gdb/ subdirectory because if
you compile the
> whole GDB with -D_FORTIFY_SOURCE=2, you get a linker
(and not
> compiler) warning. And the linker should know about
it.
>

These are all fine -- they've been addressed similarly in
gdb-6.5.

Thanks,
  Dave


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

Bernhard Walle wrote:

> Hi,
>
> * Dave Anderson <andersonredhat.com> [2007-01-17
21:10]:
> > Bernhard Walle wrote:
> > > >
> > > > 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?
> > >
> > > Of course not.  After
thinking a bit I think the correct fix would
> > > be:
> > >
> > >         buf += size_of_encoded_value(*buf);
> > >         buf++;
> > >
> > > Do you aggree?
> >
> > I can't say with any confidence.  I was trying to
correlate the difference
> > between the way it's done in gdb-6.5 vs 6.1, and
got lost doing so...
>
> Yes, but 6.5 basically does:
>
>     gdb_byte tmp = *buf++;
>     buf += size_of_encoded_value(tmp);
>
> which should be equivalent to
>
>     buf += size_of_encoded_value(*buf);
>     buf++;
>
> Of course, read_encoded_value() does more than just
calling
> size_of_encoded_value(), but anyway.
>
> > Just curious -- how do you manage to get crash to
run through that
> > code path?  (I put an exit() there, but can't seem
to get there...)
>
> I didn't. I just wrote a small C program which has the
same problem.
> (Increasing that pointer in the same operation.)
>
> Probably that code path will be never hit at all. ;)
>

Ok then, I trust your "equivalency" explanation,
and since in all
probability the code path won't be hit, it should be safe.
In fact, I don't believe that any of the other patches that
you
sent will ever be exercised by crash either?  But, if they
make
your compiler happy, that's fine with me...

>
> BTW: Is it planned to upgrade crash to use GDB 6.6 or
some higher
> version?
>

Only when and if it becomes absolutely necessary.  Every
time I've
done an upgrade, it's been a nightmare.  The hooks that are
there to
make back-door entries into the gdb code for the crash
utility's purposes
never seem to port cleanly.  And after the upgrades, there
have always
been new issues that have cropped up  -- in fact there was a
time where
I had to revert back to gdb-6.0 from gdb-6.1.

And given that the gdb module is only utilized as "gdb
vmlinux", as
long as gdb-6.1 is capable of returning structure sizes,
member offsets,
code disassembly, etc., then there's no compelling need to
upgrade it.
And if there are, the first line of defense would be to
patch the current
version than make a wholesale upgrade.

Thanks,
  Dave


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