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

Avoid really closing stdin/stdout in hclose()/hts_close()/et al #1665

Merged
merged 3 commits into from
Aug 31, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
12 changes: 10 additions & 2 deletions hfile.c
Original file line number Diff line number Diff line change
Expand Up @@ -524,7 +524,7 @@ void hclose_abruptly(hFILE *fp)
typedef struct {
hFILE base;
int fd;
unsigned is_socket:1;
unsigned is_socket:1, is_shared:1;
} hFILE_fd;

static ssize_t fd_read(hFILE *fpv, void *buffer, size_t nbytes)
Expand Down Expand Up @@ -599,6 +599,10 @@ static int fd_close(hFILE *fpv)
{
hFILE_fd *fp = (hFILE_fd *) fpv;
int ret;

// If we don't own the fd, return successfully without actually closing it
if (fp->is_shared) return 0;

do {
#ifdef HAVE_CLOSESOCKET
ret = fp->is_socket? closesocket(fp->fd) : close(fp->fd);
Expand Down Expand Up @@ -636,6 +640,7 @@ static hFILE *hopen_fd(const char *filename, const char *mode)

fp->fd = fd;
fp->is_socket = 0;
fp->is_shared = 0;
fp->base.backend = &fd_backend;
return &fp->base;

Expand Down Expand Up @@ -702,6 +707,7 @@ hFILE *hdopen(int fd, const char *mode)

fp->fd = fd;
fp->is_socket = (strchr(mode, 's') != NULL);
fp->is_shared = (strchr(mode, 'S') != NULL);
fp->base.backend = &fd_backend;
return &fp->base;
}
Expand All @@ -723,10 +729,12 @@ static hFILE *hopen_fd_fileuri(const char *url, const char *mode)
static hFILE *hopen_fd_stdinout(const char *mode)
{
int fd = (strchr(mode, 'r') != NULL)? STDIN_FILENO : STDOUT_FILENO;
char mode_shared[101];
snprintf(mode_shared, sizeof mode_shared, "S%s", mode);
#if defined HAVE_SETMODE && defined O_BINARY
if (setmode(fd, O_BINARY) < 0) return NULL;
#endif
return hdopen(fd, mode);
return hdopen(fd, mode_shared);
}

HTSLIB_EXPORT
Expand Down
15 changes: 5 additions & 10 deletions htsfile.c
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ DEALINGS IN THE SOFTWARE. */
#include <stdlib.h>
#include <string.h>
#include <getopt.h>
#include <unistd.h>

#include "htslib/hfile.h"
#include "htslib/hts.h"
Expand Down Expand Up @@ -62,13 +61,6 @@ void error(const char *format, ...)
status = EXIT_FAILURE;
}

static htsFile *dup_stdout(const char *mode)
{
int fd = dup(STDOUT_FILENO);
hFILE *hfp = (fd >= 0)? hdopen(fd, mode) : NULL;
return hfp? hts_hopen(hfp, "-", mode) : NULL;
}

static void view_sam(samFile *in, const char *filename)
{
bam1_t *b = NULL;
Expand All @@ -81,7 +73,7 @@ static void view_sam(samFile *in, const char *filename)
goto clean;
}

out = dup_stdout("w");
out = hts_open("-", "w");
if (out == NULL) { error("reopening standard output failed"); goto clean; }

if (show_headers) {
Expand Down Expand Up @@ -125,7 +117,7 @@ static void view_vcf(vcfFile *in, const char *filename)
goto clean;
}

out = dup_stdout("w");
out = hts_open("-", "w");
if (out == NULL) { error("reopening standard output failed"); goto clean; }

if (show_headers) {
Expand Down Expand Up @@ -325,5 +317,8 @@ int main(int argc, char **argv)
if (fp && hclose(fp) < 0) error("closing \"%s\" failed", argv[i]);
}

if (fclose(stdout) != 0 && errno != EBADF)
error("closing standard output failed");

return status;
}
4 changes: 4 additions & 0 deletions htslib/hfile.h
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,10 @@ Note that the file must be opened in binary mode, or else
there will be problems on platforms that make a difference
between text and binary mode.

By default, the returned hFILE "takes ownership" of the file descriptor
and _fd_ will be closed by hclose(). When _mode_ contains `S` (shared fd),
hclose() will destroy the hFILE but not close the underlying _fd_.

For socket descriptors (on Windows), _mode_ should contain `s`.
*/
HTSLIB_EXPORT
Expand Down
6 changes: 6 additions & 0 deletions test/test_view.c
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ DEALINGS IN THE SOFTWARE. */

#include <config.h>

#include <errno.h>
#include <stdio.h>
#include <unistd.h>
#include <stdlib.h>
Expand Down Expand Up @@ -430,5 +431,10 @@ int main(int argc, char *argv[])
if (p.pool)
hts_tpool_destroy(p.pool);

if (fclose(stdout) != 0 && errno != EBADF) {
fprintf(stderr, "Error closing standard output.\n");
exit_code = EXIT_FAILURE;
}

return exit_code;
}