Skip to content

Push back to dev #56

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
MaxMB15 merged 3 commits into from
Apr 6, 2026
Merged

Push back to dev #56

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
1aba7e0
fix: Linux deb updater temp file deleted before dpkg install
MaxMB15 Apr 6, 2026
cc2f9b9
fix: clean up temp file on all error paths in package_update
MaxMB15 Apr 6, 2026
6f3300f
Merge pull request #55 from MaxMB15/feature/fix-linux-egl-crash
MaxMB15 Apr 6, 2026
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
10 changes: 6 additions & 4 deletions apps/desktop/src-tauri/src/commands.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1233,14 +1233,17 @@ pub async fn package_update<R: Runtime>(app: AppHandle<R>) -> Result<(), String>

let total_size = response.content_length().unwrap_or(0);
let extension = if info.install_type == "rpm" { "rpm" } else { "deb" };
// keep() disables auto-delete and returns a TempPath (RAII: deletes on drop)
// plus the raw File handle for writing. TempPath stays alive until end of
// function, so dpkg/rpm can access the file, and cleanup happens on all paths.
Comment on lines +1236 to +1238

Copilot AI Apr 6, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These comments are inaccurate: this code uses into_parts(), not keep(). In tempfile, keep() typically disables auto-delete and returns a persisted path, while into_parts() returns a TempPath that deletes on drop. Please update the comment to describe into_parts()/TempPath behavior so future maintainers don’t misunderstand the cleanup semantics.

Suggested change
// keep() disables auto-delete and returns a TempPath (RAII: deletes on drop)
// plus the raw File handle for writing. TempPath stays alive until end of
// function, so dpkg/rpm can access the file, and cleanup happens on all paths.
// into_parts() splits the NamedTempFile into the raw File handle plus a
// TempPath. Keeping TempPath alive preserves the path for dpkg/rpm to read,
// and the temp file is still deleted automatically when TempPath is dropped.

Copilot uses AI. Check for mistakes.
use tempfile::Builder;
let temp_file = Builder::new()
.prefix("maxvideoplayer_update.")
.suffix(&format!(".{extension}"))
.tempfile_in(std::env::temp_dir())
.map_err(|e| format!("Failed to create temp file: {e}"))?;
let tmp_path = temp_file.path().to_path_buf();
let mut file = tokio::fs::File::from_std(temp_file.into_file());
let (std_file, tmp_path) = temp_file.into_parts();

Copilot AI Apr 6, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tmp_path is now a TempPath (RAII handle), not a plain PathBuf. Consider renaming to something like temp_path or temp_path_handle to reduce confusion and make the drop-based deletion behavior more obvious.

Copilot uses AI. Check for mistakes.
let mut file = tokio::fs::File::from_std(std_file);

let mut stream = response.bytes_stream();
let mut downloaded: u64 = 0;
Expand Down Expand Up @@ -1285,8 +1288,7 @@ pub async fn package_update<R: Runtime>(app: AppHandle<R>) -> Result<(), String>
_ => unreachable!(),
};

// Clean up temp file
let _ = std::fs::remove_file(&tmp_path);
// tmp_path (TempPath) auto-deletes on drop at end of function.

if !output.status.success() {
let stderr = String::from_utf8_lossy(&output.stderr);
Expand Down
2 changes: 1 addition & 1 deletion apps/desktop/src-tauri/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ fn install_crash_handler() {

let mut action: libc::sigaction = std::mem::zeroed();
action.sa_flags = libc::SA_RESETHAND;
action.sa_sigaction = crash_handler as usize;
action.sa_sigaction = crash_handler as *const () as usize;

Copilot AI Apr 6, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Casting a function to *const () and then to usize is a brittle way to populate sa_sigaction and obscures the intended ABI/type. Prefer assigning using the libc-provided handler type (e.g., libc::sighandler_t where available) or a clearly documented, platform-appropriate conversion, so this remains portable across targets where the field type/layout differs.

Suggested change
action.sa_sigaction = crash_handler as *const () as usize;
action.sa_sigaction = crash_handler as libc::sighandler_t;

Copilot uses AI. Check for mistakes.
libc::sigemptyset(&mut action.sa_mask);

libc::sigaction(libc::SIGSEGV, &action, std::ptr::null_mut());
Expand Down
Loading