Skip to content

proxy/handler: rework args handling a little bit #48274

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Oct 14, 2025
Merged

proxy/handler: rework args handling a little bit #48274

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ff1dd24
proxy/handler: rework args handling a little bit
jkarneges Oct 8, 2025
0c53a22
Merge branch 'main' into jkarneges/rework-args
jkarneges Oct 9, 2025
15bc522
Merge branch 'main' into jkarneges/rework-args
jkarneges Oct 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions src/core/config.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -426,6 +426,10 @@ impl CustomConfig {
}
}

pub fn default_config_file() -> PathBuf {
PathBuf::from(env!("CONFIG_DIR")).join("pushpin.conf")
}

pub fn get_config_file(
work_dir: &Path,
arg_config: Option<PathBuf>,
Expand All @@ -446,10 +450,7 @@ pub fn get_config_file(
.join("pushpin.conf"),
);
// default
config_files.push(PathBuf::from(format!(
"{}/pushpin.conf",
env!("CONFIG_DIR")
)));
config_files.push(default_config_file());
}
}

Expand Down
58 changes: 26 additions & 32 deletions src/handler/cliargs.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,9 @@
* limitations under the License.
*/

use crate::core::config::get_config_file;
use crate::core::config::default_config_file;
use crate::core::version;
use clap::{arg, Parser};
use std::env;
use std::ffi::CString;
use std::path::PathBuf;

Expand Down Expand Up @@ -57,17 +56,18 @@ pub struct CliArgs {
impl CliArgs {
/// Verifies the command line arguments.
pub fn verify(mut self) -> Self {
let work_dir = env::current_dir().unwrap_or_default();
let config_path: Option<PathBuf> = self.config_file.as_ref().map(PathBuf::from);

// Resolve the config file path using get_config_file
self.config_file = match get_config_file(&work_dir, config_path) {
Ok(path) => Some(path.to_string_lossy().to_string()),
Err(e) => {
eprintln!("error: failed to find configuration file: {}", e);
std::process::exit(1);
}
};
let config_file = self
.config_file
.map(PathBuf::from)
.unwrap_or_else(default_config_file);

// FIXME: don't put back as a string after resolving
self.config_file = Some(
config_file
.to_str()
.expect("path sourced from string should convert")
.to_string(),
);

if self.verbose {
self.log_level = 3;
Expand All @@ -81,12 +81,7 @@ impl CliArgs {
.config_file
.as_ref()
.map_or_else(
|| {
let work_dir = std::env::current_dir().unwrap_or_default();
let default_config = get_config_file(&work_dir, None)
.unwrap_or_else(|_| "examples/config/pushpin.conf".into());
CString::new(default_config.to_string_lossy().to_string()).unwrap()
},
|| CString::new("").unwrap(),
|s| CString::new(s.as_str()).unwrap(),
)
.into_raw();
Expand Down Expand Up @@ -120,13 +115,15 @@ impl CliArgs {
}

pub mod ffi {
use std::ffi::{c_char, c_int, c_uint};

#[repr(C)]
pub struct HandlerCliArgs {
pub config_file: *mut libc::c_char,
pub log_file: *mut libc::c_char,
pub log_level: libc::c_uint,
pub ipc_prefix: *mut libc::c_char,
pub port_offset: libc::c_int,
pub config_file: *mut c_char,
pub log_file: *mut c_char,
pub log_level: c_uint,
pub ipc_prefix: *mut c_char,
pub port_offset: c_int,
}
}

Expand Down Expand Up @@ -174,8 +171,6 @@ mod tests {
port_offset: Some(8080),
};

let args_ffi = args.to_ffi();

// Test verify() method
let verified_args = args.verify();
assert_eq!(verified_args.config_file, Some(config_test_file.clone()));
Expand All @@ -185,6 +180,8 @@ mod tests {
assert_eq!(verified_args.ipc_prefix, Some("ipc".to_string()));
assert_eq!(verified_args.port_offset, Some(8080));

let args_ffi = verified_args.to_ffi();

// Test conversion to C++-compatible struct
unsafe {
assert_eq!(
Expand Down Expand Up @@ -224,14 +221,9 @@ mod tests {
port_offset: None,
};

let empty_args_ffi = empty_args.to_ffi();

// Test verify() with empty args
let verified_empty_args = empty_args.verify();
let default_config_file = get_config_file(&env::current_dir().unwrap(), None)
.unwrap()
.to_string_lossy()
.to_string();
let default_config_file = default_config_file().to_str().unwrap().to_string();
assert_eq!(
verified_empty_args.config_file,
Some(default_config_file.clone())
Expand All @@ -242,6 +234,8 @@ mod tests {
assert_eq!(verified_empty_args.ipc_prefix, None);
assert_eq!(verified_empty_args.port_offset, None);

let empty_args_ffi = verified_empty_args.to_ffi();

// Test conversion to C++-compatible struct
unsafe {
assert_eq!(
Expand Down
9 changes: 5 additions & 4 deletions src/handler/handlerapp.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,9 +104,6 @@ class HandlerApp::Private
QCoreApplication::setApplicationVersion(Config::get().version);

HandlerArgsData args(argsFfi);
Settings settings(args.configFile);
if (!args.ipcPrefix.isEmpty()) settings.setIpcPrefix(args.ipcPrefix);
if (args.portOffset != -1) settings.setPortOffset(args.portOffset);

// Set the log level
if(args.logLevel != -1)
Expand All @@ -131,11 +128,15 @@ class HandlerApp::Private
QFile file(args.configFile);
if(!file.open(QIODevice::ReadOnly))
{
log_error("failed to open %s", qPrintable(args.configFile));
log_error("failed to open config file: %s", qPrintable(args.configFile));
return 1;
}
}

Settings settings(args.configFile);
if (!args.ipcPrefix.isEmpty()) settings.setIpcPrefix(args.ipcPrefix);
if (args.portOffset != -1) settings.setPortOffset(args.portOffset);

QStringList services = settings.value("runner/services").toStringList();

QStringList connmgr_in_stream_specs = settings.value("proxy/connmgr_in_stream_specs").toStringList();
Expand Down
120 changes: 58 additions & 62 deletions src/proxy/cliargs.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,12 @@
* limitations under the License.
*/

use crate::core::config::get_config_file;
use crate::core::config::default_config_file;
use crate::core::version;
use clap::{arg, Parser};
use std::env;
use std::ffi::CString;
use std::ffi::{c_char, CString};
use std::path::PathBuf;
use std::slice;

// Struct to hold the command line arguments
#[derive(Parser, Debug)]
Expand Down Expand Up @@ -57,17 +57,18 @@ pub struct CliArgs {
impl CliArgs {
/// Verifies the command line arguments.
pub fn verify(mut self) -> Self {
let work_dir = env::current_dir().unwrap_or_default();
let config_path: Option<PathBuf> = self.config_file.as_ref().map(PathBuf::from);

// Resolve the config file path using get_config_file
self.config_file = match get_config_file(&work_dir, config_path) {
Ok(path) => Some(path.to_string_lossy().to_string()),
Err(e) => {
eprintln!("error: failed to find configuration file: {}", e);
std::process::exit(1);
}
};
let config_file = self
.config_file
.map(PathBuf::from)
.unwrap_or_else(default_config_file);

// FIXME: don't put back as a string after resolving
self.config_file = Some(
config_file
.to_str()
.expect("path sourced from string should convert")
.to_string(),
);

if self.verbose {
self.log_level = 3;
Expand All @@ -81,12 +82,7 @@ impl CliArgs {
.config_file
.as_ref()
.map_or_else(
|| {
let work_dir = std::env::current_dir().unwrap_or_default();
let default_config = get_config_file(&work_dir, None)
.unwrap_or_else(|_| "examples/config/pushpin.conf".into());
CString::new(default_config.to_string_lossy().to_string()).unwrap()
},
|| CString::new("").unwrap(),
|s| CString::new(s.as_str()).unwrap(),
)
.into_raw();
Expand All @@ -109,47 +105,47 @@ impl CliArgs {
)
.into_raw();

let (routes, routes_count) = if !self.route.is_empty() {
// Allocate array of string pointers
let routes_array = unsafe {
libc::malloc(self.route.len() * std::mem::size_of::<*mut libc::c_char>())
as *mut *mut libc::c_char
};

// Convert each route to CString and store pointer in array
for (i, item) in self.route.iter().enumerate() {
let c_string = CString::new(item.to_string()).unwrap().into_raw();
unsafe {
*routes_array.add(i) = c_string;
}
}

(routes_array, self.route.len() as libc::c_uint)
} else {
let routes_array = unsafe { libc::malloc(0) as *mut *mut libc::c_char };
(routes_array, 0)
};
let mut routes: Vec<*mut c_char> = Vec::new();

// Convert each route to CString and store pointer in array
for item in &self.route {
let item = item.as_bytes();

// ensure no trailing nul byte
let end = item.iter().position(|b| *b == 0).unwrap_or(item.len());
let item = &item[..end];

let c_string = CString::new(item).unwrap().into_raw();
routes.push(c_string);
}

let routes = routes.into_boxed_slice();

let routes_count = routes.len();
let routes: *mut [*mut c_char] = Box::into_raw(routes);

ffi::ProxyCliArgs {
config_file,
log_file,
log_level: self.log_level,
ipc_prefix,
routes,
routes: routes as *mut *mut c_char,
routes_count,
}
}
}

pub mod ffi {
use std::ffi::{c_char, c_uint};

#[repr(C)]
pub struct ProxyCliArgs {
pub config_file: *mut libc::c_char,
pub log_file: *mut libc::c_char,
pub log_level: libc::c_uint,
pub ipc_prefix: *mut libc::c_char,
pub routes: *mut *mut libc::c_char,
pub routes_count: libc::c_uint,
pub config_file: *mut c_char,
pub log_file: *mut c_char,
pub log_level: c_uint,
pub ipc_prefix: *mut c_char,
pub routes: *mut *mut c_char,
pub routes_count: libc::size_t,
}
}

Expand Down Expand Up @@ -177,15 +173,18 @@ pub unsafe extern "C" fn destroy_proxy_cli_args(ffi_args: ffi::ProxyCliArgs) {
let _ = CString::from_raw(ffi_args.ipc_prefix);
}
if !ffi_args.routes.is_null() {
// SAFETY: the raw parts were originally derived from a boxed slice
let routes = unsafe {
Box::from_raw(slice::from_raw_parts_mut(
ffi_args.routes,
ffi_args.routes_count,
))
};

// Free each individual route string
for i in 0..ffi_args.routes_count {
let route_ptr = *ffi_args.routes.add(i as usize);
if !route_ptr.is_null() {
let _ = CString::from_raw(route_ptr);
}
for item in routes.iter() {
let _ = CString::from_raw(*item);
}

libc::free(ffi_args.routes as *mut libc::c_void);
}
}

Expand All @@ -209,8 +208,6 @@ mod tests {
route: vec!["route1".to_string(), "route2".to_string()],
};

let args_ffi = args.to_ffi();

// Test verify() method
let verified_args = args.verify();
assert_eq!(verified_args.config_file, Some(config_test_file.clone()));
Expand All @@ -223,6 +220,8 @@ mod tests {
vec!["route1".to_string(), "route2".to_string()]
);

let args_ffi = verified_args.to_ffi();

// Test conversion to C++-compatible struct
unsafe {
assert_eq!(
Expand Down Expand Up @@ -277,14 +276,9 @@ mod tests {
route: Vec::new(),
};

let empty_args_ffi = empty_args.to_ffi();

// Test verify() with empty args
let verified_empty_args = empty_args.verify();
let default_config_file = get_config_file(&env::current_dir().unwrap(), None)
.unwrap()
.to_string_lossy()
.to_string();
let default_config_file = default_config_file().to_str().unwrap().to_string();
assert_eq!(
verified_empty_args.config_file,
Some(default_config_file.clone())
Expand All @@ -295,6 +289,8 @@ mod tests {
assert_eq!(verified_empty_args.ipc_prefix, None);
assert!(verified_empty_args.route.is_empty());

let empty_args_ffi = verified_empty_args.to_ffi();

// Test conversion to C++-compatible struct
unsafe {
assert_eq!(
Expand Down
Loading