Skip to content
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.

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

cp: avoid a resource leak #1238

Merged
merged 1 commit into from
May 23, 2024
Merged

Conversation

khorben
Copy link
Contributor

@khorben khorben commented May 16, 2024

In copy_file(), make sure the from_fd file descriptor is closed even when the operation failed early.

Reported by: Coverity Scan
CID: 1545036
Sponsored by: The FreeBSD Foundation

@@ -112,6 +112,8 @@ copy_file(const FTSENT *entp, int dne)
if ((from_fd = open(entp->fts_path, O_RDONLY, 0)) < 0 ||
fstat(from_fd, &sb) != 0) {
warn("%s", entp->fts_path);
if (from_fd >= 0)
Copy link
Member

Choose a reason for hiding this comment

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

You could just close it since you're casting away the return value anyway.

Copy link
Member

Choose a reason for hiding this comment

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

I'd rather avoid calling close with an invalid fd, and am happy with the change as proposed

Copy link
Member

@bsdimp bsdimp May 23, 2024

Choose a reason for hiding this comment

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

OK. It would disturb errno, but the warn also could as well... but we're not super careful with errno in this code

In copy_file(), make sure the from_fd file descriptor is closed even
when the operation failed early.

Reported by:	Coverity Scan
CID:		1545036
Sponsored by:	The FreeBSD Foundation

Reviewed by: imp, emaste
Pull Request: freebsd#1238
@freebsd-git freebsd-git merged commit 3d7c8f0 into freebsd:main May 23, 2024
7 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants