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
