Skip to content

Commit

Permalink
OpamStd.Sys.{get_terminal_columns,uname,getconf,guess_shell_compat}: …
Browse files Browse the repository at this point in the history
…Harden the process calls to account for failure

with_process_in was fairly brittle and would display stderr by default
and ignore errors. The new version of that function (renamed process_in)
now uses Unix.open_process_full and returns None if anything went wrong,
fixing the issue on OpenBSD where `getconf LONG_BIT` would fail.
  • Loading branch information
kit-ty-kate committed Oct 9, 2024
1 parent 5ad463d commit 19246ac
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 30 deletions.
2 changes: 2 additions & 0 deletions master_changes.md
Original file line number Diff line number Diff line change
Expand Up @@ -126,3 +126,5 @@ users)
## opam-format

## opam-core
* `OpamStd.Sys.{get_terminal_columns,uname,getconf,guess_shell_compat}`: Harden the process calls to account for failures [#6230 @kit-ty-kate - fix #6215]
* `OpamStd.Sys.{uname,getconf}`: now accepts only one argument as parameter, as per their documentation [#6230 @kit-ty-kate]
63 changes: 33 additions & 30 deletions src/core/opamStd.ml
Original file line number Diff line number Diff line change
Expand Up @@ -903,19 +903,23 @@ module OpamSys = struct
let split = if clean then OpamString.split else OpamString.split_delim in
split path path_sep

let with_process_in cmd args f =
let process_in cmd args =
if Sys.win32 then
assert false;
let path = split_path_variable (Env.get "PATH") in
let cmd =
List.find Sys.file_exists (List.map (fun d -> Filename.concat d cmd) path)
in
let ic = Unix.open_process_in (cmd^" "^args) in
let env = Env.raw_env () in
try
let r = f ic in
ignore (Unix.close_process_in ic) ; r
with exn ->
ignore (Unix.close_process_in ic) ; raise exn
let path = split_path_variable (Env.get "PATH") in
let cmd =
List.find OpamStubs.is_executable
(List.map (fun d -> Filename.concat d cmd) path)
in
let args = Array.of_list (cmd :: args) in
let (ic, _, _) as p = Unix.open_process_args_full cmd args env in
let r = input_line ic in
match Unix.close_process_full p with
| Unix.WEXITED 0 -> Some r
| WEXITED _ | WSIGNALED _ | WSTOPPED _ -> None
with Unix.Unix_error _ | Sys_error _ | End_of_file | Not_found -> None

let tty_out = Unix.isatty Unix.stdout

Expand All @@ -934,20 +938,21 @@ module OpamSys = struct
let get_terminal_columns () =
let fallback = 80 in
let cols =
try (* terminfo *)
with_process_in "tput" "cols"
(fun ic -> int_of_string (input_line ic))
match (* terminfo *)
Option.replace int_of_string_opt (process_in "tput" ["cols"])
with
| Unix.Unix_error _ | Sys_error _ | Failure _ | End_of_file | Not_found ->
| Some x -> x
| None ->
try (* GNU stty *)
with_process_in "stty" "size"
(fun ic ->
match OpamString.split (input_line ic) ' ' with
| [_ ; v] -> int_of_string v
| _ -> failwith "stty")
begin match
Option.map (fun x -> OpamString.split x ' ')
(process_in "stty" ["size"])
with
| Some [_ ; v] -> int_of_string v
| _ -> failwith "stty"
end
with
| Unix.Unix_error _ | Sys_error _ | Failure _
| End_of_file | Not_found -> fallback
| Failure _ -> fallback
in
if cols > 0 then cols else fallback

Expand Down Expand Up @@ -1000,10 +1005,9 @@ module OpamSys = struct
fun cmd arg ->
try Hashtbl.find memo (cmd, arg) with Not_found ->
let r =
try
with_process_in cmd arg
(fun ic -> Some (OpamString.strip (input_line ic)))
with Unix.Unix_error _ | Sys_error _ | Not_found -> None
match process_in cmd [arg] with
| None -> None
| Some x -> Some (OpamString.strip x)
in
Hashtbl.add memo (cmd, arg) r;
r
Expand Down Expand Up @@ -1119,12 +1123,11 @@ module OpamSys = struct
Some (Unix.readlink (Filename.concat dir "exe"))
with e ->
fatal e;
try
with_process_in "ps"
(Printf.sprintf "-p %d -o comm= 2>/dev/null" ppid)
(fun ic -> Some (input_line ic))
match
process_in "ps" ["-p"; string_of_int ppid; "-o"; "comm="]
with
| Unix.Unix_error _ | Sys_error _ | Failure _ | End_of_file | Not_found ->
| Some _ as x -> x
| None ->
try
let c = open_in_bin ("/proc/" ^ string_of_int ppid ^ "/cmdline") in
begin try
Expand Down

0 comments on commit 19246ac

Please sign in to comment.