Re: [Patch, microblaze]: Add slr and shr regs and little-endian breakpoint

[Michael Eager <eager at eagercon dot com>]

On 05/22/14 22:57, Eli Zaretskii wrote:
> > Date: Thu, 22 May 2014 12:44:45 -0700
> > From: Michael Eager <eager@eagerm.com>
> > CC: Michael Eager <eager@eagercon.com>,  "gdb-patches@sourceware.org" <gdb-patches@sourceware.org>, Vinod Kathail <vinodk@xilinx.com>,  Vidhumouli Hunsigida <vidhum@xilinx.com>, Nagaraju Mekala <nmekala@xilinx.com>
> >
> > > Based on the feedback, the updated patch is given below.
> > >
> > > Okay for the upstream?
> >
> > An OK would only be appropriate if you had write access to the repository.
>
> An OK is appropriate even if the OP doesn't have write access.  An OK
> just means the patch is approved for committing.
>
> > >       This patch add the support of slr and shr regs and also solves the problem
> >
> > also?  Is there another unmentioned problem?
>
> There is indeed another problem, but it _is_ mentioned: the missing
> support for slr and shr registers.
>
> > >       related to process_g_packet where the buf_len > 2 * rsa->sizeof_g_packet
> > >       and throwing the Error that 'g' packet message reply is too long. This is
> > >       because the buf_len calculated in the init_remote_state function for
> > >       microblaze target is based On the sizeof_g_packet and remote_packet_size
> > >       and the memory_packet_config->size. The sizeof_g_packet is 236 because the
> > >       number of reg num is 59 and 2* sizeof_g_packet comes to 472 .With shr and
> > >       shl entry and the buf_len is 472. This does not match the greater than
> > >       conditional statement  and works fine. Without shr and shl entry,the
> > >       sizeof_g_packets comes to 57*4 *2 = 456.  This doesn't match the criteria
> > >       in the process_g_packet function  leading to throwing of error message as
> > >       " 'g' packet message reply is too long".
> >
> > Please make the description of the problem reasonably succinct.  A detailed
> > analysis of how you identified the problem is not needed, especially when you
> > mention the error message and the cause of the error multiple times.
>
> I see no reason to object to detailed descriptions like the one above
> from the POV of their length.  They don't hurt, and aren't terribly
> long to begin with.

Eli -- The problem  with this description is that it doesn't describe
the actual problem, but more describes the debugging process.  Describing
the if statement which tests the packet length doesn't tell what caused
it to be incorrect.


> > What is running on the target?
> >
> > If this a problem using the XMD gdbserver stub which is returning more registers
> > than GDB expects?  If that is the case, then say so.  Otherwise, what is the cause
> > of the mismatch between the gdb G packet and the target?
>
> If you want to suggest a rewording, it is much better IME to just show
> precisely your suggestion.  After all, for most of us here (present
> parties included) English is not our first language, maybe not even
> the second.

I'm happy to suggest better wording, but at the moment I have no more
than a guess what the root cause of the problem is.  Ajit says my guess is
wrong, and he didn't respond to my questions about what is running on
the target which does cause the packet length mismatch.


--
Michael Eager	 eager@eagercon.com
1960 Park Blvd., Palo Alto, CA 94306  650-325-8077