Re: New ARI warning Sat Mar 19 01:54:11 UTC 2011

[Pedro Alves <pedro@codesourcery.com>]

On Wednesday 23 March 2011 17:16:01, Joel Brobecker wrote:
> > This was a large diff, but in fact, there is only one new warning:
> > 
> > > gdb/i386-tdep.c:1693: obsolete: frame_register_read: Replace frame_register_read() with get_frame_register(), or possibly introduce a new method safe_get_frame_register()
> > gdb/i386-tdep.c:1693:     && frame_register_read (this_frame, cache->saved_sp_reg, buf))
> 
> I just had a look at this ARI warning.  The comment on
> frame_register_read says:
> 
> /* FIXME: cagney/2003-02-02: Should be deprecated or replaced with a
>    function called get_frame_register_p().  This slightly weird (and
>    older) variant of get_frame_register() returns zero (indicating the
>    register value is unavailable/invalid) if either: the register
>    isn't cached; or the register has been optimized out; or the
>    register contents are unavailable (because they haven't been
>    collected in a traceframe).  Problem is, neither check is exactly
>    correct.  A register can't be optimized out (it may not have been
>    saved as part of a function call); The fact that a register isn't
>    in the register cache doesn't mean that the register isn't
>    available (it could have been fetched from memory).  */
> 
> I have had this feeling that we have way too many ways to read/write
> frame registers, but I'm wondering if this comment might not be
> too zealous in this case. This function seems useful, because it
> returns a status as opposed to get_frame_register, which has the exact
> same profile except that it throws instead of returning. So I'm thinking
> we should remove the "deprecation" fixme, and just keep the FIXME for
> fixing whatever incorrectness might be left, and then remove this from
> the ARI.
> 
> Thoughts?

Agreed.

-- 
Pedro Alves