Skip to content

Commit 778543b

Browse files
hebastoluke-jr
andauthored
Avoid leaking POSIX name aliases beyond subprocess.hpp (#112)
The commit a32c0f3 introduced code to silence MSVC's "warning C4996: The POSIX name for this item is deprecated." However, it exhibits several issues: 1. The aliases may leak into code outside the `subprocess.hpp` header. 2. They are unnecessarily applied when using the MinGW-w64 toolchain. 3. The fix is incomplete: downstream projects still see C4996 warnings. 4. The implementation lacks documentation. This change addresses all of the above shortcomings. Co-authored-by: Luke Dashjr <luke-jr+git@utopios.org>
1 parent 34033d0 commit 778543b

1 file changed

Lines changed: 46 additions & 34 deletions

File tree

subprocess.hpp

Lines changed: 46 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -64,10 +64,6 @@ extern "C" {
6464
#include <windows.h>
6565
#include <io.h>
6666
#include <cwchar>
67-
68-
#define close _close
69-
#define open _open
70-
#define fileno _fileno
7167
#else
7268
#include <sys/wait.h>
7369
#include <unistd.h>
@@ -77,6 +73,22 @@ extern "C" {
7773
#include <sys/types.h>
7874
}
7975

76+
// The Microsoft C++ compiler issues deprecation warnings
77+
// for the standard POSIX function names.
78+
// Its preferred implementations have a leading underscore.
79+
// See: https://learn.microsoft.com/en-us/cpp/c-runtime-library/compatibility.
80+
#if (defined _MSC_VER)
81+
#define subprocess_close _close
82+
#define subprocess_fileno _fileno
83+
#define subprocess_open _open
84+
#define subprocess_write _write
85+
#else
86+
#define subprocess_close close
87+
#define subprocess_fileno fileno
88+
#define subprocess_open open
89+
#define subprocess_write write
90+
#endif
91+
8092
/*!
8193
* Getting started with reading this source code.
8294
* The source is mainly divided into four parts:
@@ -281,7 +293,7 @@ namespace util
281293

282294
FILE *fp = _fdopen(os_fhandle, mode);
283295
if (fp == 0) {
284-
_close(os_fhandle);
296+
subprocess_close(os_fhandle);
285297
throw OSError("_fdopen", 0);
286298
}
287299

@@ -514,7 +526,7 @@ namespace util
514526
{
515527
size_t nwritten = 0;
516528
while (nwritten < length) {
517-
int written = write(fd, buf + nwritten, length - nwritten);
529+
int written = subprocess_write(fd, buf + nwritten, length - nwritten);
518530
if (written == -1) return -1;
519531
nwritten += written;
520532
}
@@ -542,7 +554,7 @@ namespace util
542554
#ifdef __USING_WINDOWS__
543555
return (int)fread(buf, 1, read_upto, fp);
544556
#else
545-
int fd = fileno(fp);
557+
int fd = subprocess_fileno(fp);
546558
int rbytes = 0;
547559
int eintr_cnter = 0;
548560

@@ -783,10 +795,10 @@ struct input
783795
explicit input(int fd): rd_ch_(fd) {}
784796

785797
// FILE pointer.
786-
explicit input (FILE* fp):input(fileno(fp)) { assert(fp); }
798+
explicit input (FILE* fp):input(subprocess_fileno(fp)) { assert(fp); }
787799

788800
explicit input(const char* filename) {
789-
int fd = open(filename, O_RDONLY);
801+
int fd = subprocess_open(filename, O_RDONLY);
790802
if (fd == -1) throw OSError("File not found: ", errno);
791803
rd_ch_ = fd;
792804
}
@@ -816,10 +828,10 @@ struct output
816828
{
817829
explicit output(int fd): wr_ch_(fd) {}
818830

819-
explicit output (FILE* fp):output(fileno(fp)) { assert(fp); }
831+
explicit output (FILE* fp):output(subprocess_fileno(fp)) { assert(fp); }
820832

821833
explicit output(const char* filename) {
822-
int fd = open(filename, O_APPEND | O_CREAT | O_RDWR, 0640);
834+
int fd = subprocess_open(filename, O_APPEND | O_CREAT | O_RDWR, 0640);
823835
if (fd == -1) throw OSError("File not found: ", errno);
824836
wr_ch_ = fd;
825837
}
@@ -847,10 +859,10 @@ struct error
847859
{
848860
explicit error(int fd): wr_ch_(fd) {}
849861

850-
explicit error(FILE* fp):error(fileno(fp)) { assert(fp); }
862+
explicit error(FILE* fp):error(subprocess_fileno(fp)) { assert(fp); }
851863

852864
explicit error(const char* filename) {
853-
int fd = open(filename, O_APPEND | O_CREAT | O_RDWR, 0640);
865+
int fd = subprocess_open(filename, O_APPEND | O_CREAT | O_RDWR, 0640);
854866
if (fd == -1) throw OSError("File not found: ", errno);
855867
wr_ch_ = fd;
856868
}
@@ -1105,28 +1117,28 @@ class Streams
11051117
void cleanup_fds()
11061118
{
11071119
if (write_to_child_ != -1 && read_from_parent_ != -1) {
1108-
close(write_to_child_);
1120+
subprocess_close(write_to_child_);
11091121
}
11101122
if (write_to_parent_ != -1 && read_from_child_ != -1) {
1111-
close(read_from_child_);
1123+
subprocess_close(read_from_child_);
11121124
}
11131125
if (err_write_ != -1 && err_read_ != -1) {
1114-
close(err_read_);
1126+
subprocess_close(err_read_);
11151127
}
11161128
}
11171129

11181130
void close_parent_fds()
11191131
{
1120-
if (write_to_child_ != -1) close(write_to_child_);
1121-
if (read_from_child_ != -1) close(read_from_child_);
1122-
if (err_read_ != -1) close(err_read_);
1132+
if (write_to_child_ != -1) subprocess_close(write_to_child_);
1133+
if (read_from_child_ != -1) subprocess_close(read_from_child_);
1134+
if (err_read_ != -1) subprocess_close(err_read_);
11231135
}
11241136

11251137
void close_child_fds()
11261138
{
1127-
if (write_to_parent_ != -1) close(write_to_parent_);
1128-
if (read_from_parent_ != -1) close(read_from_parent_);
1129-
if (err_write_ != -1) close(err_write_);
1139+
if (write_to_parent_ != -1) subprocess_close(write_to_parent_);
1140+
if (read_from_parent_ != -1) subprocess_close(read_from_parent_);
1141+
if (err_write_ != -1) subprocess_close(err_write_);
11301142
}
11311143

11321144
FILE* input() { return input_.get(); }
@@ -1613,8 +1625,8 @@ inline void Popen::execute_process() noexcept(false)
16131625
child_pid_ = fork();
16141626

16151627
if (child_pid_ < 0) {
1616-
close(err_rd_pipe);
1617-
close(err_wr_pipe);
1628+
subprocess_close(err_rd_pipe);
1629+
subprocess_close(err_wr_pipe);
16181630
throw OSError("fork failed", errno);
16191631
}
16201632

@@ -1626,14 +1638,14 @@ inline void Popen::execute_process() noexcept(false)
16261638
stream_.close_parent_fds();
16271639

16281640
//Close the read end of the error pipe
1629-
close(err_rd_pipe);
1641+
subprocess_close(err_rd_pipe);
16301642

16311643
detail::Child chld(this, err_wr_pipe);
16321644
chld.execute_child();
16331645
}
16341646
else
16351647
{
1636-
close (err_wr_pipe);// close child side of pipe, else get stuck in read below
1648+
subprocess_close(err_wr_pipe);// close child side of pipe, else get stuck in read below
16371649

16381650
stream_.close_child_fds();
16391651

@@ -1642,7 +1654,7 @@ inline void Popen::execute_process() noexcept(false)
16421654

16431655
FILE* err_fp = fdopen(err_rd_pipe, "r");
16441656
if (!err_fp) {
1645-
close(err_rd_pipe);
1657+
subprocess_close(err_rd_pipe);
16461658
throw OSError("fdopen failed", errno);
16471659
}
16481660
int read_bytes = util::read_atmost_n(err_fp, err_buf, SP_MAX_ERR_BUF_SIZ);
@@ -1765,13 +1777,13 @@ namespace detail {
17651777

17661778
// Close the duped descriptors
17671779
if (stream.read_from_parent_ != -1 && stream.read_from_parent_ > 2)
1768-
close(stream.read_from_parent_);
1780+
subprocess_close(stream.read_from_parent_);
17691781

17701782
if (stream.write_to_parent_ != -1 && stream.write_to_parent_ > 2)
1771-
close(stream.write_to_parent_);
1783+
subprocess_close(stream.write_to_parent_);
17721784

17731785
if (stream.err_write_ != -1 && stream.err_write_ > 2)
1774-
close(stream.err_write_);
1786+
subprocess_close(stream.err_write_);
17751787

17761788
// Close all the inherited fd's except the error write pipe
17771789
if (parent_->close_fds_) {
@@ -1780,7 +1792,7 @@ namespace detail {
17801792

17811793
for (int i = 3; i < max_fd; i++) {
17821794
if (i == err_wr_pipe_) continue;
1783-
close(i);
1795+
subprocess_close(i);
17841796
}
17851797
}
17861798

@@ -1831,15 +1843,15 @@ namespace detail {
18311843
#ifdef __USING_WINDOWS__
18321844
util::configure_pipe(&this->g_hChildStd_IN_Rd, &this->g_hChildStd_IN_Wr, &this->g_hChildStd_IN_Wr);
18331845
this->input(util::file_from_handle(this->g_hChildStd_IN_Wr, "w"));
1834-
this->write_to_child_ = _fileno(this->input());
1846+
this->write_to_child_ = subprocess_fileno(this->input());
18351847

18361848
util::configure_pipe(&this->g_hChildStd_OUT_Rd, &this->g_hChildStd_OUT_Wr, &this->g_hChildStd_OUT_Rd);
18371849
this->output(util::file_from_handle(this->g_hChildStd_OUT_Rd, "r"));
1838-
this->read_from_child_ = _fileno(this->output());
1850+
this->read_from_child_ = subprocess_fileno(this->output());
18391851

18401852
util::configure_pipe(&this->g_hChildStd_ERR_Rd, &this->g_hChildStd_ERR_Wr, &this->g_hChildStd_ERR_Rd);
18411853
this->error(util::file_from_handle(this->g_hChildStd_ERR_Rd, "r"));
1842-
this->err_read_ = _fileno(this->error());
1854+
this->err_read_ = subprocess_fileno(this->error());
18431855
#else
18441856

18451857
if (write_to_child_ != -1) input(fdopen(write_to_child_, "wb"));

0 commit comments

Comments
 (0)