Re: [Qemu-devel] [PATCH] PATCH1 - infrastructure for Evaluate breakpoint condition on target.OPC_MAX_SIZE - size of translation buffer - was increased because condition translated bytecode should be executed in one block

[Jan Kiszka]

On 2013-02-27 08:25, Neiman, Anna wrote:
> Hi Jan,
> Thank you for your review.
> I added 4 patches including whole implementation of this feature, not just 
> infrastructure.
> 
> Some explanations for your notes :

Please comment inline, do not top-post.

> Cond_exp is the array of bytecodes ( not string ), cond_len - size of this 
> array . For more info ,please, look GDB agent documentation: 
> http://sourceware.org/gdb/onlinedocs/gdb/Agent-Expressions.html#Agent-Expressions

OK, thanks for clarifying.

> Cond_exp is removed on breakpoint release.
> In all cases when we have error in breakpoint condition, it just will be 
> evaluated on host ( the old mode ).

Not sure what you mean with "host" here, but syntax errors in the remote
gdb command (maybe not the condition bytecode) shall be evaluated before
injecting the breakpoint and shall be reported back to the gdb frontend,
not the local QEMU console.

> Relating checks for spaces - maybe it is not needed but it seems me more safe 
> and it doesn't require  huge resources.

Spaces are clearly against the spec and shall not be accepted by QEMU's
gdbstub.

> 
> Please, let me know if you see critical issues , which prevent the 
> confirmation of the patch.
> The issues listed below are not look those.

All issues are important unless you can convince us that they aren't.
You should try to adopt all the suggestions we make quickly and
carefully or explain very well where you don't. Then repost new versions
of the series. That will help making the patches acceptable for
upstream. The current form is not yet. But that's normal, specifically
when you start working in a new community.

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
Corporate Competence Center Embedded Linux