Skip to content

Target --remote#4

Merged
Cerdore merged 4 commits intoCerdore:mainfrom
jon-hirst:target-remote
Apr 24, 2026
Merged

Target --remote#4
Cerdore merged 4 commits intoCerdore:mainfrom
jon-hirst:target-remote

Conversation

@jon-hirst
Copy link
Copy Markdown
Contributor

Description

Add the ability to connect to a GDB server with: --remote hostname:port.

Fixed 2 bugs that I found during development:

e5fb262 - Remove json.dumps() from session_meta

e05bd3e - Execute commands on the Python main thread, not the thread that receives from socket.

Type of Change

  • Bug fix
  • New feature
  • Documentation update
  • Refactoring

Testing

  • Tests pass locally
  • New tests added for new functionality

I apologize that I do not understand how the tests are written.

Checklist

  • Code follows project style guidelines
  • Documentation updated if needed
  • Commit messages are clear

@Cerdore
Copy link
Copy Markdown
Owner

Cerdore commented Apr 24, 2026

Code Review: Target --remote Remote Debugging Support

Thanks for this PR! The overall design is solid - adding remote debugging capability plus fixing two important bugs. Here are the issues I found:


🔴 Must Fix

1. Duplicate key in JSON output (cli.py lines 236-242)

print_json({
    "session_id": session.session_id,
    "mode": session.mode,
    "remote": session.remote,      # First occurrence
    "binary": session.binary,
    "remote": session.remote,      # ← DUPLICATE! Remove this line
    "sock_path": session.sock_path,
    ...
})

2. Error status not handled (handlers.py)

output_status, output = result_queue.get(timeout=30.0)

return {
    "command": command,
    "output": output or "(no output)"
}

output_status is retrieved but never checked. If the command throws an exception internally, the tuple would be ("error", str(e)), but this isn't handled - errors would be returned as normal output.

Suggested fix:

if output_status == "error":
    return {"error": output, "command": command}

3. Type annotation mismatch (session.py line 53)

remote: Optional[int] = None,  # Should be Optional[str]

The parameter is described as host:port (string), but annotated as int. This will fail static type checking.


🟡 Suggestions

4. Missing mi-async on for target mode (launcher.py)

In launch_attach():

gdb_commands.append("set non-stop on")
gdb_commands.append("set target-async on")
gdb_commands.append("set mi-async on")  # ← This line exists

But launch_target() only has:

gdb_commands.append("set non-stop on")
gdb_commands.append("set target-async on")
# Missing: "set mi-async on"

This may cause inconsistent async behavior for remote targets in non-stop mode.


5. Queue timeout without exception handling (handlers.py)

output_status, output = result_queue.get(timeout=30.0)

After 30 seconds, queue.Empty is raised but not caught in the try block. Complex commands (loading large symbols, etc.) may timeout and cause unhandled exceptions.

Suggested fix:

try:
    output_status, output = result_queue.get(timeout=60.0)
except queue.Empty:
    return {"error": "Command timeout", "command": command}

6. Remote address format not validated

--remote host:port has no format validation before being passed to GDB.

Suggested addition:

import re
if not re.match(r'^[\w.-]+:\d+$', remote):
    raise GDBLauncherError(f"Invalid remote format: {remote}")

Summary

Please fix issues 1-3 before merging. Items 4-6 are suggested improvements.

🤖 Generated with Claude Code

@Cerdore Cerdore merged commit 8e432dd into Cerdore:main Apr 24, 2026
0 of 5 checks passed
Cerdore added a commit that referenced this pull request Apr 24, 2026
- Remove duplicate "remote" key in target JSON output
- Check output_status for error in handle_exec
- Fix remote type annotation from Optional[int] to Optional[str]
- Add missing mi-async on for target mode non-stop
- Handle queue.Empty timeout explicitly with 60s timeout
- Add remote address format validation (host:port)

Constraint: Must validate remote format before passing to GDB
Confidence: high
Scope-risk: narrow

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants