Skip to content

close_fds: fix close of external fds#241

Merged
haircommander merged 1 commit intocontainers:masterfrom
giuseppe:fix-close-fds
Mar 1, 2021
Merged

close_fds: fix close of external fds#241
haircommander merged 1 commit intocontainers:masterfrom
giuseppe:fix-close-fds

Conversation

@giuseppe
Copy link
Member

commit 0f092d5 introduced the
regression.

Closes: #240

Signed-off-by: Giuseppe Scrivano gscrivan@redhat.com

@giuseppe
Copy link
Member Author

CC @baude

@haircommander
Copy link
Collaborator

LGTM, @giuseppe did you test against podman unit/system tests? since we don't run them on CI it has to be manually done (for now..)

@giuseppe
Copy link
Member Author

giuseppe commented Mar 1, 2021

LGTM, @giuseppe did you test against podman unit/system tests? since we don't run them on CI it has to be manually done (for now..)

I've ran the e2e tests and they pass locally

src/close_fds.c Outdated

for (fd = 3; fd < open_files_max_fd; fd++) {
for (fd = 3; fd <= open_files_max_fd; fd++) {
if (open_files_set == NULL || FD_ISSET(fd % FD_SETSIZE, &(open_files_set[fd / FD_SETSIZE])))
Copy link
Collaborator

Choose a reason for hiding this comment

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

sorry, one more quick nit, can we return from this function early if open_files_set == NULL?

Copy link
Member Author

Choose a reason for hiding this comment

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

good idea!

Pushed new version

commit 0f092d5 introduced the
regression.

Closes: containers#240

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
@haircommander haircommander merged commit 3ac015e into containers:master Mar 1, 2021
@haircommander
Copy link
Collaborator

thanks @giuseppe , I assume we want a release for this @baude ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

podman restart with port bindings results in 'port in use' errors

2 participants