From 31bbefee33fdf5c7c68f569059795338b6424a80 Mon Sep 17 00:00:00 2001 From: Alexander Kiselev Date: Thu, 5 Feb 2026 12:07:52 -0800 Subject: [PATCH] fix: address code review issues across codebase - Make --quiet flag functional by threading it through execute_via_bridge() - Fix --sort hyphen ambiguity with allow_hyphen_values in clap - Replace deprecated atty crate with std::io::IsTerminal - Use config.default_limit as fallback when user doesn't specify --limit - Respect per-command --json flag in QueryOptions format detection - Remove dead src/daemon/ module and src/ipc/transport.rs - Deduplicate headless_script lookup (GhidraClient delegates to bridge) - Fix stop_bridge() SIGTERM race with graceful shutdown wait loop - Extend config set to support all config keys - Clean up dead code warnings and remove blanket #![allow(dead_code)] Co-Authored-By: Claude Opus 4.6 --- CLAUDE.md | 2 -- Cargo.lock | 21 ------------- Cargo.toml | 1 - src/cli.rs | 4 +-- src/config.rs | 16 ---------- src/daemon/README.md | 71 -------------------------------------------- src/daemon/mod.rs | 71 -------------------------------------------- src/error.rs | 27 +---------------- src/filter/mod.rs | 24 --------------- src/format/mod.rs | 12 -------- src/ghidra/bridge.rs | 44 +++++++-------------------- src/ghidra/mod.rs | 48 ++---------------------------- src/ghidra/setup.rs | 1 + src/ipc/README.md | 1 - src/ipc/mod.rs | 1 - src/ipc/transport.rs | 14 --------- src/main.rs | 68 +++++++++++++++++++++++++++++------------- src/query/mod.rs | 7 ++++- 18 files changed, 71 insertions(+), 362 deletions(-) delete mode 100644 src/daemon/README.md delete mode 100644 src/daemon/mod.rs delete mode 100644 src/ipc/transport.rs diff --git a/CLAUDE.md b/CLAUDE.md index c1dbf0e..835fd38 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -24,7 +24,6 @@ See @AGENTS.md for agent-specific instructions. |------|------| | `src/ghidra/` | Bridge management, Ghidra setup/installation, Java bridge script | | `src/ipc/` | TCP client, protocol definitions, transport helpers | -| `src/daemon/` | Thin wrapper over bridge.rs (kept for API compatibility) | | `src/format/` | Handling output format conversion (Table, Compact, JSON, CSV, etc.) | | `tests/` | Writing integration or unit tests | @@ -33,7 +32,6 @@ See @AGENTS.md for agent-specific instructions. | What | When | |------|------| | `CHANGELOG.md` | Reviewing version history and release notes | -| `src/daemon/README.md` | Understanding daemon wrapper and its delegation to bridge.rs | | `src/ghidra/README.md` | Understanding bridge lifecycle, PID file sequence, TOCTOU elimination, BridgeClient adoption | | `src/ipc/README.md` | Understanding TCP wire format, BridgeClient API, single implementation rationale | | `tests/README.md` | Understanding test structure and conventions | diff --git a/Cargo.lock b/Cargo.lock index dc1cc9b..3a0ad8b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -108,17 +108,6 @@ dependencies = [ "wait-timeout", ] -[[package]] -name = "atty" -version = "0.2.14" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d9b39be18770d11421cdb1b9947a45dd3f37e93092cbf377614828a319d5fee8" -dependencies = [ - "hermit-abi", - "libc", - "winapi", -] - [[package]] name = "autocfg" version = "1.5.0" @@ -754,7 +743,6 @@ version = "0.1.3" dependencies = [ "anyhow", "assert_cmd", - "atty", "chrono", "clap", "comfy-table", @@ -831,15 +819,6 @@ version = "0.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2304e00983f87ffb38b55b444b5e3b60a884b5d30c0fca7d82fe33449bbe55ea" -[[package]] -name = "hermit-abi" -version = "0.1.19" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "62b467343b94ba476dcb2500d242dadbb39557df889310ac77c5d99100aaac33" -dependencies = [ - "libc", -] - [[package]] name = "hmac" version = "0.12.1" diff --git a/Cargo.toml b/Cargo.toml index 88a4ae8..dedf97d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -65,7 +65,6 @@ strsim = "0.11" # Cross-platform support dunce = "1.0" # Windows path handling -atty = "0.2" # TTY detection # Setup command dependencies reqwest = { version = "0.11", features = ["json", "stream", "rustls-tls"] } diff --git a/src/cli.rs b/src/cli.rs index c3b62a4..c84c93a 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -209,7 +209,7 @@ pub struct QueryArgs { pub offset: Option, /// Sort by field(s) (comma-separated, prefix with - for descending) - #[arg(long)] + #[arg(long, allow_hyphen_values = true)] pub sort: Option, /// Only return count @@ -797,7 +797,7 @@ pub struct QueryOptions { #[arg(long)] pub offset: Option, - #[arg(long)] + #[arg(long, allow_hyphen_values = true)] pub sort: Option, #[arg(long)] diff --git a/src/config.rs b/src/config.rs index 1b56c50..ccaf9d2 100644 --- a/src/config.rs +++ b/src/config.rs @@ -1,5 +1,3 @@ -#![allow(dead_code)] - use crate::error::{GhidraError, Result}; use serde::{Deserialize, Serialize}; use std::fs; @@ -158,25 +156,11 @@ impl Config { None } - pub fn get_timeout(&self) -> u64 { - std::env::var("GHIDRA_TIMEOUT") - .ok() - .and_then(|s| s.parse().ok()) - .or(self.timeout) - .unwrap_or(300) - } - pub fn get_default_program(&self) -> Option { std::env::var("GHIDRA_DEFAULT_PROGRAM") .ok() .or_else(|| self.default_program.clone()) } - - pub fn get_default_project(&self) -> Option { - std::env::var("GHIDRA_DEFAULT_PROJECT") - .ok() - .or_else(|| self.default_project.clone()) - } } #[cfg(test)] diff --git a/src/daemon/README.md b/src/daemon/README.md deleted file mode 100644 index 326898b..0000000 --- a/src/daemon/README.md +++ /dev/null @@ -1,71 +0,0 @@ -# Bridge Management Module (`src/daemon/`) - -Thin compatibility wrapper over `src/ghidra/bridge.rs`. All logic lives in `bridge.rs`; this module re-exports functions for callers that still reference `daemon::*`. - -## Why This Module Exists - -Historical: the original architecture had a separate Rust daemon process. Now the "daemon" is just the long-running Java bridge process. This module was kept to avoid breaking the `daemon::` import paths used by some callers and tests. - -## API Surface - -| Function | Delegates To | -|----------|-------------| -| `ensure_bridge(config, mode)` | `bridge::ensure_bridge_running()` | -| `start_bridge(config, mode)` | `bridge::start_bridge()` | -| `stop_bridge(project_path)` | `bridge::stop_bridge()` | -| `get_bridge_status(project_path)` | `bridge::bridge_status()` | -| `is_bridge_running(project_path) -> Option` | `bridge::is_bridge_running()` | - -`is_bridge_running` returns `Option` (the port) instead of `bool`. Callers use the returned port directly, eliminating TOCTOU races between liveness checks and port file reads. - -## Architecture - -See `src/ghidra/README.md` for the canonical bridge architecture documentation, including: - -- Bridge lifecycle (spawn, PID write, ready signal, port file) -- Liveness detection (port file + PID alive + BridgeClient ping) -- Auto-start behavior for Import, Quick, and Analyze commands -- TOCTOU elimination via `is_bridge_running() -> Option` - -## Liveness Detection - -Bridge liveness is verified in `bridge::is_bridge_running()` and `bridge::bridge_status()`: - -1. **Port file exists** -- `bridge-{hash}.port` in `~/.local/share/ghidra-cli/` -2. **PID alive** -- `kill(pid, 0)` succeeds on Unix -3. **TCP reachable** -- `TcpStream::connect("127.0.0.1:{port}")` succeeds (in `is_bridge_running`) -4. **Ping verified** -- `BridgeClient::new(port).ping()` returns true (in `bridge_status` and `verify_bridge`) - -`is_bridge_running` uses a raw TCP probe (step 3) for speed. `bridge_status` uses BridgeClient ping (step 4) for stronger verification. The `verify_bridge()` helper in `main.rs` is called after connecting to an existing bridge to confirm it responds to commands. - -## Command Flow - -All commands flow through the OutputFormat formatter in `main.rs`: - -1. Import/Quick produce structured `serde_json::Value` directly -2. Analyze and query commands go through `execute_via_bridge()` which returns `serde_json::Value` -3. The formatter at the end of `run_with_bridge()` applies Table, JSON, or JsonCompact formatting -4. Progress messages use `eprintln!()` (stderr), structured output uses `println!()` (stdout) - -## Auto-Start Behavior - -| Command | Bridge Not Running | Bridge Running | -|---------|-------------------|----------------| -| Import | Start bridge in Import mode | Import via `client.import_binary()`, switch program | -| Quick | Start bridge in Import mode, analyze | Import via running bridge, switch program, analyze | -| Analyze | Start bridge in Process mode, analyze via `execute_via_bridge` | Analyze via `execute_via_bridge` | -| Query commands | Start bridge in Process mode | Connect and query | - -Quick reuses a running bridge by importing through the bridge's `import` command and switching to the new program, rather than starting a fresh bridge in Import mode. - -Analyze has no special-case handler. It falls through to the generic dispatch path in `execute_via_bridge()`. - -## Key Differences from Original Architecture - -- No separate Rust daemon process; the Java bridge IS the persistent server -- `BridgeClient` (in `src/ipc/client.rs`) is the single TCP command implementation; bridge.rs uses raw `TcpStream` only for lightweight connect probes, not for sending commands -- `is_bridge_running()` returns `Option` (port), not `bool` -- `bridge_status()` uses `BridgeClient.ping()` instead of raw `TcpStream::connect` -- `stop_bridge()` uses `BridgeClient.shutdown()` for graceful shutdown -- Rust writes PID file immediately after spawn (before Java ready signal) -- Failed starts kill orphaned child processes and clean up stale files diff --git a/src/daemon/mod.rs b/src/daemon/mod.rs deleted file mode 100644 index c1e54ad..0000000 --- a/src/daemon/mod.rs +++ /dev/null @@ -1,71 +0,0 @@ -//! Bridge management module. -//! -//! Manages the lifecycle of the Java GhidraCliBridge process. -//! The "daemon" is just the long-running Ghidra/Java bridge process - -//! there is no separate Rust daemon. The CLI connects directly to -//! the bridge via TCP. - -use std::path::{Path, PathBuf}; - -use anyhow::Result; - -use crate::ghidra::bridge::{self, BridgeStartMode, BridgeStatus}; - -/// Bridge configuration (replaces old DaemonConfig). -#[allow(dead_code)] -pub struct BridgeConfig { - /// Path to the Ghidra project directory - pub project_path: PathBuf, - /// Ghidra installation directory - pub ghidra_install_dir: PathBuf, -} - -/// Ensure a bridge is running for the given project. -/// If import mode, starts with the binary. If process mode, opens existing program. -/// Returns the port number for connecting. -#[allow(dead_code)] -pub fn ensure_bridge(config: &BridgeConfig, mode: BridgeStartMode) -> Result { - bridge::ensure_bridge_running(&config.project_path, &config.ghidra_install_dir, mode) -} - -/// Start a new bridge for the given project. -/// Returns the port number for connecting. -#[allow(dead_code)] -pub fn start_bridge(config: &BridgeConfig, mode: BridgeStartMode) -> Result { - bridge::start_bridge(&config.project_path, &config.ghidra_install_dir, mode) -} - -/// Stop the bridge for a project. -#[allow(dead_code)] -pub fn stop_bridge(project_path: &Path) -> Result<()> { - bridge::stop_bridge(project_path) -} - -/// Get bridge status for a project. -#[allow(dead_code)] -pub fn get_bridge_status(project_path: &Path) -> Result { - bridge::bridge_status(project_path) -} - -/// Check if a bridge is running for a project. -/// Returns `Some(port)` if running, `None` otherwise. -#[allow(dead_code)] -pub fn is_bridge_running(project_path: &Path) -> Option { - bridge::is_bridge_running(project_path) -} - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn test_bridge_config() { - let config = BridgeConfig { - project_path: PathBuf::from("/test/project"), - ghidra_install_dir: PathBuf::from("/opt/ghidra"), - }; - - assert_eq!(config.project_path, PathBuf::from("/test/project")); - assert_eq!(config.ghidra_install_dir, PathBuf::from("/opt/ghidra")); - } -} diff --git a/src/error.rs b/src/error.rs index a6621a8..837dc0c 100644 --- a/src/error.rs +++ b/src/error.rs @@ -1,5 +1,3 @@ -#![allow(dead_code)] - use thiserror::Error; #[derive(Error, Debug)] @@ -7,27 +5,16 @@ pub enum GhidraError { #[error("Ghidra installation not found. Set GHIDRA_INSTALL_DIR or run 'ghidra init'")] GhidraNotFound, - #[error("Ghidra project not found: {0}")] - ProjectNotFound(String), - - #[error("Program not found: {0}")] - ProgramNotFound(String), - - #[error("Failed to execute Ghidra: {0}")] - ExecutionFailed(String), - #[error("Failed to parse filter: {0}")] FilterParseError(String), #[error("Invalid filter expression: {0}")] InvalidFilter(String), - #[error("Field not found: {0}")] - FieldNotFound(String), - #[error("Invalid format: {0}")] InvalidFormat(String), + #[allow(dead_code)] #[error("Invalid data type: {0}")] InvalidDataType(String), @@ -42,18 +29,6 @@ pub enum GhidraError { #[error("YAML error: {0}")] YamlError(#[from] serde_yaml::Error), - - #[error("Command failed: {0}")] - CommandFailed(String), - - #[error("Invalid address: {0}")] - InvalidAddress(String), - - #[error("Analysis timeout after {0} seconds")] - Timeout(u64), - - #[error("{0}")] - Other(String), } pub type Result = std::result::Result; diff --git a/src/filter/mod.rs b/src/filter/mod.rs index dac403a..f8bd34b 100644 --- a/src/filter/mod.rs +++ b/src/filter/mod.rs @@ -71,15 +71,7 @@ pub enum Value { Hex(u64), } -#[allow(dead_code)] impl Value { - pub fn as_str(&self) -> Option<&str> { - match self { - Value::String(s) => Some(s), - _ => None, - } - } - pub fn as_f64(&self) -> Option { match self { Value::Number(n) => Some(*n), @@ -88,22 +80,6 @@ impl Value { _ => None, } } - - pub fn as_i64(&self) -> Option { - match self { - Value::Integer(i) => Some(*i), - Value::Number(n) => Some(*n as i64), - Value::Hex(h) => Some(*h as i64), - _ => None, - } - } - - pub fn as_bool(&self) -> Option { - match self { - Value::Boolean(b) => Some(*b), - _ => None, - } - } } pub struct Filter { diff --git a/src/format/mod.rs b/src/format/mod.rs index 3d3a0d7..b594283 100644 --- a/src/format/mod.rs +++ b/src/format/mod.rs @@ -4,7 +4,6 @@ use serde::Serialize; use serde_json::Value as JsonValue; #[derive(Debug, Clone, Copy, PartialEq, Eq)] -#[allow(dead_code)] pub enum OutputFormat { Full, Compact, @@ -23,7 +22,6 @@ pub enum OutputFormat { C, } -#[allow(dead_code)] impl OutputFormat { pub fn from_str(s: &str) -> Result { match s.to_lowercase().as_str() { @@ -46,16 +44,6 @@ impl OutputFormat { } } - pub fn is_human_friendly(&self) -> bool { - matches!(self, Self::Full | Self::Compact | Self::Table | Self::Tree) - } - - pub fn is_machine_friendly(&self) -> bool { - matches!( - self, - Self::Json | Self::JsonCompact | Self::JsonStream | Self::Csv | Self::Tsv - ) - } } pub trait Formatter { diff --git a/src/ghidra/bridge.rs b/src/ghidra/bridge.rs index 45fbe7f..2a100ac 100644 --- a/src/ghidra/bridge.rs +++ b/src/ghidra/bridge.rs @@ -7,7 +7,7 @@ use std::io::{BufRead, BufReader}; use std::net::TcpStream; use std::path::{Path, PathBuf}; -use std::process::{Child, Command, Stdio}; +use std::process::{Command, Stdio}; use std::time::Duration; use anyhow::{Context, Result}; @@ -327,8 +327,16 @@ pub fn stop_bridge(project_path: &Path) -> Result<()> { } } - // If PID file exists, kill the process as fallback + // Wait for the process to exit gracefully, then force-kill if needed if let Ok(Some(pid)) = read_pid_file(project_path) { + // Wait up to 3 seconds for graceful exit + for _ in 0..30 { + if !is_pid_alive(pid) { + break; + } + std::thread::sleep(Duration::from_millis(100)); + } + // If still alive after waiting, force kill if is_pid_alive(pid) { warn!("Killing bridge process {} as fallback", pid); #[cfg(unix)] @@ -388,7 +396,7 @@ pub enum BridgeStatus { } /// Find the analyzeHeadless script. -fn find_headless_script(ghidra_install_dir: &Path) -> Result { +pub fn find_headless_script(ghidra_install_dir: &Path) -> Result { let support_dir = ghidra_install_dir.join("support"); #[cfg(unix)] @@ -405,33 +413,3 @@ fn find_headless_script(ghidra_install_dir: &Path) -> Result { } } -/// Convenience wrapper for wait_timeout on Child -trait ChildExt { - fn wait_timeout( - &mut self, - timeout: Duration, - ) -> std::io::Result>; -} - -impl ChildExt for Child { - fn wait_timeout( - &mut self, - timeout: Duration, - ) -> std::io::Result> { - use std::thread; - use std::time::Instant; - - let start = Instant::now(); - loop { - match self.try_wait()? { - Some(status) => return Ok(Some(status)), - None => { - if start.elapsed() >= timeout { - return Ok(None); - } - thread::sleep(Duration::from_millis(100)); - } - } - } - } -} diff --git a/src/ghidra/mod.rs b/src/ghidra/mod.rs index dfe9594..d7d6f8b 100644 --- a/src/ghidra/mod.rs +++ b/src/ghidra/mod.rs @@ -1,5 +1,3 @@ -#![allow(dead_code)] - pub mod bridge; pub mod setup; @@ -9,7 +7,6 @@ use std::path::{Path, PathBuf}; #[derive(Debug)] pub struct GhidraClient { - config: Config, install_dir: PathBuf, project_dir: PathBuf, } @@ -25,37 +22,14 @@ impl GhidraClient { } Ok(Self { - config, install_dir, project_dir, }) } - pub fn install_dir(&self) -> &PathBuf { - &self.install_dir - } - - pub fn get_headless_script(&self) -> PathBuf { - let support_dir = self.install_dir.join("support"); - - #[cfg(target_os = "windows")] - { - support_dir.join("analyzeHeadless.bat") - } - - #[cfg(not(target_os = "windows"))] - { - support_dir.join("analyzeHeadless") - } - } - pub fn verify_installation(&self) -> Result<()> { - let headless = self.get_headless_script(); - - if !headless.exists() { - return Err(GhidraError::GhidraNotFound); - } - + bridge::find_headless_script(&self.install_dir) + .map_err(|_| GhidraError::GhidraNotFound)?; Ok(()) } @@ -82,24 +56,6 @@ impl GhidraClient { Ok(()) } - fn get_scripts_dir(&self) -> Result { - let config_dir = dirs::config_dir().ok_or_else(|| { - GhidraError::ConfigError("Could not determine config directory".to_string()) - })?; - - let scripts_dir = config_dir.join("ghidra-cli").join("scripts"); - - if !scripts_dir.exists() { - std::fs::create_dir_all(&scripts_dir)?; - } - - Ok(scripts_dir) - } - - pub fn get_install_dir(&self) -> &Path { - &self.install_dir - } - pub fn get_project_dir(&self) -> &Path { &self.project_dir } diff --git a/src/ghidra/setup.rs b/src/ghidra/setup.rs index 25e2cec..2e9457c 100644 --- a/src/ghidra/setup.rs +++ b/src/ghidra/setup.rs @@ -8,6 +8,7 @@ use std::path::{Path, PathBuf}; /// GitHub release asset information #[derive(Deserialize, Debug)] +#[allow(dead_code)] struct GithubAsset { name: String, browser_download_url: String, diff --git a/src/ipc/README.md b/src/ipc/README.md index 4409ac2..9ac3090 100644 --- a/src/ipc/README.md +++ b/src/ipc/README.md @@ -8,7 +8,6 @@ Single TCP implementation for CLI-to-bridge communication. All command sending g |------|---------| | `client.rs` | `BridgeClient` -- the canonical client for all bridge commands | | `protocol.rs` | `BridgeRequest` / `BridgeResponse` wire format structs | -| `transport.rs` | Utility: `port_reachable()` TCP probe | ## BridgeClient diff --git a/src/ipc/mod.rs b/src/ipc/mod.rs index 96f8544..b424959 100644 --- a/src/ipc/mod.rs +++ b/src/ipc/mod.rs @@ -5,4 +5,3 @@ pub mod client; pub mod protocol; -pub mod transport; diff --git a/src/ipc/transport.rs b/src/ipc/transport.rs deleted file mode 100644 index 0d69121..0000000 --- a/src/ipc/transport.rs +++ /dev/null @@ -1,14 +0,0 @@ -//! Transport helpers for bridge TCP communication. -//! -//! The Java bridge uses newline-delimited JSON over TCP. -//! This module provides minimal transport utilities. - -use std::net::TcpStream; - -/// Check if a TCP port is reachable on localhost. -#[allow(dead_code)] -pub fn port_reachable(port: u16) -> bool { - TcpStream::connect(format!("127.0.0.1:{}", port)) - .map(|_| true) - .unwrap_or(false) -} diff --git a/src/main.rs b/src/main.rs index 2a5b97c..68b0553 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1,6 +1,5 @@ mod cli; mod config; -mod daemon; mod error; mod filter; mod format; @@ -17,6 +16,7 @@ use ghidra::bridge::{self, BridgeStartMode, BridgeStatus}; use ghidra::GhidraClient; use ipc::client::BridgeClient; use query::Query; +use std::io::IsTerminal; use std::path::PathBuf; use tracing_subscriber::layer::SubscriberExt; use tracing_subscriber::util::SubscriberInitExt; @@ -443,7 +443,9 @@ fn run_with_bridge(cli: Cli) -> anyhow::Result<()> { // Switch to the newly imported program client.open_program(&program_name)?; - eprintln!("Successfully imported as: {}", program_name); + if !cli.quiet { + eprintln!("Successfully imported as: {}", program_name); + } json!({ "command": "import", "program": program_name, @@ -452,7 +454,9 @@ fn run_with_bridge(cli: Cli) -> anyhow::Result<()> { }) } else { // No bridge running - start one in import mode - eprintln!("Starting Ghidra bridge..."); + if !cli.quiet { + eprintln!("Starting Ghidra bridge..."); + } let port = bridge::ensure_bridge_running( &project_path, &ghidra_install_dir, @@ -468,7 +472,9 @@ fn run_with_bridge(cli: Cli) -> anyhow::Result<()> { .unwrap_or("unknown") .to_string() }); - eprintln!("Successfully imported as: {}", program_name); + if !cli.quiet { + eprintln!("Successfully imported as: {}", program_name); + } json!({ "command": "import", "program": program_name, @@ -496,13 +502,17 @@ fn run_with_bridge(cli: Cli) -> anyhow::Result<()> { BridgeStartMode::Project }; - eprintln!("Starting Ghidra bridge..."); + if !cli.quiet { + eprintln!("Starting Ghidra bridge..."); + } let port = bridge::ensure_bridge_running( &project_path, &ghidra_install_dir, mode, )?; - eprintln!("Bridge ready."); + if !cli.quiet { + eprintln!("Bridge ready."); + } BridgeClient::new(port) }; @@ -521,12 +531,14 @@ fn run_with_bridge(cli: Cli) -> anyhow::Result<()> { } } - execute_via_bridge(&client, &cli.command)? + execute_via_bridge(&client, &cli.command, cli.quiet, config.default_limit)? } }; // Check for .NET decompilation and warn - check_dotnet_decompile_warning(&cli.command, &result); + if !cli.quiet { + check_dotnet_decompile_warning(&cli.command, &result); + } // Determine output format: explicit -o flag > --json/--pretty > TTY detection let opts = extract_query_options(&cli.command); @@ -542,10 +554,10 @@ fn run_with_bridge(cli: Cli) -> anyhow::Result<()> { fmt } else if cli.pretty { OutputFormat::Json - } else if cli.json { + } else if cli.json || opts.as_ref().map_or(false, |o| o.json) { OutputFormat::JsonCompact } else { - auto_detect_format(atty::is(atty::Stream::Stdout)) + auto_detect_format(std::io::stdout().is_terminal()) }; // Unwrap bridge response envelopes before formatting @@ -574,15 +586,21 @@ fn run_with_bridge(cli: Cli) -> anyhow::Result<()> { fn execute_via_bridge( client: &BridgeClient, command: &Commands, + quiet: bool, + default_limit: Option, ) -> anyhow::Result { use serde_json::json; match command { // Analyze shares the generic dispatch path with all query commands Commands::Analyze(_) => { - eprintln!("Analyzing..."); + if !quiet { + eprintln!("Analyzing..."); + } let result = client.analyze()?; - eprintln!("Analysis complete!"); + if !quiet { + eprintln!("Analysis complete!"); + } Ok(json!({ "command": "analyze", "status": "success", @@ -590,8 +608,8 @@ fn execute_via_bridge( })) } Commands::Query(args) => match args.data_type.as_str() { - "functions" => client.list_functions(args.limit, None), - "strings" => client.list_strings(args.limit), + "functions" => client.list_functions(args.limit.or(default_limit), None), + "strings" => client.list_strings(args.limit.or(default_limit)), "imports" => client.list_imports(), "exports" => client.list_exports(), "memory" => client.memory_map(), @@ -602,7 +620,7 @@ fn execute_via_bridge( use cli::FunctionCommands; match cmd { FunctionCommands::List(opts) => { - client.list_functions(opts.limit, None) + client.list_functions(opts.limit.or(default_limit), None) } FunctionCommands::Decompile(args) => client.decompile(args.target.clone()), FunctionCommands::Get(args) => { @@ -636,7 +654,7 @@ fn execute_via_bridge( Commands::Strings(cmd) => { use cli::StringsCommands; match cmd { - StringsCommands::List(opts) => client.list_strings(opts.limit), + StringsCommands::List(opts) => client.list_strings(opts.limit.or(default_limit)), StringsCommands::Refs(args) => client.xrefs_to(args.string.clone()), } } @@ -672,9 +690,9 @@ fn execute_via_bridge( DumpCommands::Imports(_) => client.list_imports(), DumpCommands::Exports(_) => client.list_exports(), DumpCommands::Functions(opts) => { - client.list_functions(opts.limit, None) + client.list_functions(opts.limit.or(default_limit), None) } - DumpCommands::Strings(opts) => client.list_strings(opts.limit), + DumpCommands::Strings(opts) => client.list_strings(opts.limit.or(default_limit)), } } Commands::Summary(_) => client.program_info(), @@ -725,7 +743,7 @@ fn execute_via_bridge( Commands::Type(cmd) => { use cli::TypeCommands; match cmd { - TypeCommands::List(opts) => client.type_list(opts.limit), + TypeCommands::List(opts) => client.type_list(opts.limit.or(default_limit)), TypeCommands::Get(args) => client.type_get(&args.name), TypeCommands::Create(args) => client.type_create(&args.definition), TypeCommands::Apply(args) => client.type_apply(&args.address, &args.type_name), @@ -745,7 +763,7 @@ fn execute_via_bridge( Commands::Graph(cmd) => { use cli::GraphCommands; match cmd { - GraphCommands::Calls(opts) => client.graph_calls(opts.limit), + GraphCommands::Calls(opts) => client.graph_calls(opts.limit.or(default_limit)), GraphCommands::Callers(args) => client.graph_callers(&args.function, args.depth), GraphCommands::Callees(args) => client.graph_callees(&args.function, args.depth), GraphCommands::Export(args) => client.graph_export(&args.format), @@ -1134,6 +1152,16 @@ fn handle_config_command(cmd: cli::ConfigCommands) -> anyhow::Result<()> { })?; config.timeout = Some(timeout); } + "ghidra_install_dir" => config.ghidra_install_dir = Some(PathBuf::from(value)), + "ghidra_project_dir" => config.ghidra_project_dir = Some(PathBuf::from(value)), + "default_program" => config.default_program = Some(value), + "default_project" => config.default_project = Some(value), + "default_limit" => { + let limit: usize = value.parse().map_err(|_| { + GhidraError::ConfigError("Invalid limit value".to_string()) + })?; + config.default_limit = Some(limit); + } _ => { anyhow::bail!("Unknown config key: {}", key); } diff --git a/src/query/mod.rs b/src/query/mod.rs index 3bde34b..91b66d4 100644 --- a/src/query/mod.rs +++ b/src/query/mod.rs @@ -63,8 +63,8 @@ pub struct Query { pub count_only: bool, } -#[allow(dead_code)] impl Query { + #[allow(dead_code)] pub fn new(data_type: DataType) -> Self { Self { data_type, @@ -114,26 +114,31 @@ impl Query { })) } + #[allow(dead_code)] pub fn with_filter(mut self, filter: Filter) -> Self { self.filter = Some(filter); self } + #[allow(dead_code)] pub fn with_format(mut self, format: OutputFormat) -> Self { self.format = format; self } + #[allow(dead_code)] pub fn with_limit(mut self, limit: usize) -> Self { self.limit = Some(limit); self } + #[allow(dead_code)] pub fn with_offset(mut self, offset: usize) -> Self { self.offset = Some(offset); self } + #[allow(dead_code)] pub fn count_only(mut self) -> Self { self.count_only = true; self