[Lldb-commits] [PATCH] Patch for fixing the handling of hardware breakpoints

[Greg Clayton]

Looks good.

On Feb 21, 2014, at 10:19 AM, Deepak Panickal <deepak2427 at gmail.com> wrote:

>  Fix the comments
> 
> http://llvm-reviews.chandlerc.com/D2826
> 
> CHANGE SINCE LAST DIFF
>  http://llvm-reviews.chandlerc.com/D2826?vs=7278&id=7287#toc
> 
> Files:
>  source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
> 
> Index: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
> ===================================================================
> --- source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
> +++ source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
> @@ -2305,18 +2305,18 @@
>     Error error;
>     assert(bp_site != NULL);
> 
> -    // get logging info
> +    // Get logging info
>     Log *log(ProcessGDBRemoteLog::GetLogIfAllCategoriesSet(GDBR_LOG_BREAKPOINTS));
>     user_id_t site_id = bp_site->GetID();
> 
> -    // get the breakpoint address
> +    // Get the breakpoint address
>     const addr_t addr = bp_site->GetLoadAddress();
> 
> -    // log that a breakpoint was requested
> +    // Log that a breakpoint was requested
>     if (log)
>         log->Printf("ProcessGDBRemote::EnableBreakpointSite (size_id = %" PRIu64 ") address = 0x%" PRIx64, site_id, (uint64_t)addr);
> 
> -    // breakpoint already exists and is enabled
> +    // Breakpoint already exists and is enabled
>     if (bp_site->IsEnabled())
>     {
>         if (log)
> @@ -2324,7 +2324,7 @@
>         return error;
>     }
> 
> -    // get the software breakpoint trap opcode size
> +    // Get the software breakpoint trap opcode size
>     const size_t bp_op_size = GetSoftwareBreakpointTrapOpcode(bp_site);
> 
>     // SupportsGDBStoppointPacket() simply checks a boolean, indicating if this breakpoint type
> @@ -2336,10 +2336,10 @@
>     // skip over software breakpoints.
>     if (m_gdb_comm.SupportsGDBStoppointPacket(eBreakpointSoftware) && (!bp_site->HardwareRequired()))
>     {
> -        // try to send off a software breakpoint packet ($Z0)
> +        // Try to send off a software breakpoint packet ($Z0)
>         if (m_gdb_comm.SendGDBStoppointTypePacket(eBreakpointSoftware, true, addr, bp_op_size) == 0)
>         {
> -            // the breakpoint was placed successfully
> +            // The breakpoint was placed successfully
>             bp_site->SetEnabled(true);
>             bp_site->SetType(BreakpointSite::eExternal);
>             return error;
> @@ -2355,52 +2355,52 @@
>         if (m_gdb_comm.SupportsGDBStoppointPacket(eBreakpointSoftware))
>             return error;
> 
> -        // we reach here when software breakpoints have been found to be unsupported. For future
> +        // We reach here when software breakpoints have been found to be unsupported. For future
>         // calls to set a breakpoint, we will not attempt to set a breakpoint with a type that is
>         // known not to be supported.
>         if (log)
>             log->Printf("Software breakpoints are unsupported");
> 
> -        // so we will fall through and try a hardware breakpoint
> +        // So we will fall through and try a hardware breakpoint
>     }
> 
> -    // the process of setting a hardware breakpoint is much the same as above.  We check the
> +    // The process of setting a hardware breakpoint is much the same as above.  We check the
>     // supported boolean for this breakpoint type, and if it is thought to be supported then we
>     // will try to set this breakpoint with a hardware breakpoint.
>     if (m_gdb_comm.SupportsGDBStoppointPacket(eBreakpointHardware))
>     {
> -        // try to send off a hardware breakpoint packet ($Z1)
> +        // Try to send off a hardware breakpoint packet ($Z1)
>         if (m_gdb_comm.SendGDBStoppointTypePacket(eBreakpointHardware, true, addr, bp_op_size) == 0)
>         {
> -            // the breakpoint was placed successfully
> +            // The breakpoint was placed successfully
>             bp_site->SetEnabled(true);
>             bp_site->SetType(BreakpointSite::eHardware);
>             return error;
>         }
> 
> -        // check if the error was something other then an unsupported breakpoint type
> +        // Check if the error was something other than an unsupported breakpoint type
>         if (m_gdb_comm.SupportsGDBStoppointPacket(eBreakpointHardware))
>         {
> -            // unable to set this hardware breakpoint
> -            error.SetErrorString("failed to set hardware breakpoint (hardware breakpoint resources might be exhausted or unavailable)");
> +            // Unable to set this hardware breakpoint
> +            error.SetErrorString("failed to set hardware breakpoint (hardware breakpoint resources might be exhausted)");
>             return error;
>         }
> 
> -        // we will reach here when the stub gives an unsported response to a hardware breakpoint
> +        // We will reach here when the stub gives an unsupported response to a hardware breakpoint
>         if (log)
>             log->Printf("Hardware breakpoints are unsupported");
> 
> -        // finally we will falling through to a #trap style breakpoint
> +        // Finally we will falling through to a #trap style breakpoint
>     }
> 
> -    // don't fall through when hardware breakpoints were specifically requested
> +    // Don't fall through when hardware breakpoints were specifically requested
>     if (bp_site->HardwareRequired())
>     {
>         error.SetErrorString("hardware breakpoints are not supported");
>         return error;
>     }
> 
> -    // as a last resort we want to place a manual breakpoint. an instruction
> +    // As a last resort we want to place a manual breakpoint. an instruction
>     // is placed into the process memory using memory write packets
>     return EnableSoftwareBreakpoint(bp_site);
> }
> <D2826.3.patch>_______________________________________________
> lldb-commits mailing list
> lldb-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits