Skip to content

Commit 174bd43

Browse files
hebastoluke-jr
andcommitted
subprocess: Avoid leaking POSIX name aliases beyond subprocess.h
The commit a32c0f3df4b6bcd1d2e93f19e8f380bb890cd507 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. Github-Pull: arun11299/cpp-subprocess#112 Rebased-From: 778543b2f2ca7f5d1c4f0241b635bbb265d750dd Co-authored-by: Luke Dashjr <luke-jr+git@utopios.org>
1 parent 7997b76 commit 174bd43

File tree

1 file changed

+45
-33
lines changed

1 file changed

+45
-33
lines changed

src/util/subprocess.h

Lines changed: 45 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -68,10 +68,6 @@ extern "C" {
6868
#include <windows.h>
6969
#include <io.h>
7070
#include <cwchar>
71-
72-
#define close _close
73-
#define open _open
74-
#define fileno _fileno
7571
#else
7672
#include <sys/wait.h>
7773
#include <unistd.h>
@@ -81,6 +77,22 @@ extern "C" {
8177
#include <sys/types.h>
8278
}
8379

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

265277
FILE *fp = _fdopen(os_fhandle, mode);
266278
if (fp == 0) {
267-
_close(os_fhandle);
279+
subprocess_close(os_fhandle);
268280
throw OSError("_fdopen", 0);
269281
}
270282

@@ -383,7 +395,7 @@ namespace util
383395
{
384396
size_t nwritten = 0;
385397
while (nwritten < length) {
386-
int written = write(fd, buf + nwritten, length - nwritten);
398+
int written = subprocess_write(fd, buf + nwritten, length - nwritten);
387399
if (written == -1) return -1;
388400
nwritten += written;
389401
}
@@ -411,7 +423,7 @@ namespace util
411423
#ifdef __USING_WINDOWS__
412424
return (int)fread(buf, 1, read_upto, fp);
413425
#else
414-
int fd = fileno(fp);
426+
int fd = subprocess_fileno(fp);
415427
int rbytes = 0;
416428
int eintr_cnter = 0;
417429

@@ -573,10 +585,10 @@ struct input
573585
explicit input(int fd): rd_ch_(fd) {}
574586

575587
// FILE pointer.
576-
explicit input (FILE* fp):input(fileno(fp)) { assert(fp); }
588+
explicit input (FILE* fp):input(subprocess_fileno(fp)) { assert(fp); }
577589

578590
explicit input(const char* filename) {
579-
int fd = open(filename, O_RDONLY);
591+
int fd = subprocess_open(filename, O_RDONLY);
580592
if (fd == -1) throw OSError("File not found: ", errno);
581593
rd_ch_ = fd;
582594
}
@@ -606,10 +618,10 @@ struct output
606618
{
607619
explicit output(int fd): wr_ch_(fd) {}
608620

609-
explicit output (FILE* fp):output(fileno(fp)) { assert(fp); }
621+
explicit output (FILE* fp):output(subprocess_fileno(fp)) { assert(fp); }
610622

611623
explicit output(const char* filename) {
612-
int fd = open(filename, O_APPEND | O_CREAT | O_RDWR, 0640);
624+
int fd = subprocess_open(filename, O_APPEND | O_CREAT | O_RDWR, 0640);
613625
if (fd == -1) throw OSError("File not found: ", errno);
614626
wr_ch_ = fd;
615627
}
@@ -637,10 +649,10 @@ struct error
637649
{
638650
explicit error(int fd): wr_ch_(fd) {}
639651

640-
explicit error(FILE* fp):error(fileno(fp)) { assert(fp); }
652+
explicit error(FILE* fp):error(subprocess_fileno(fp)) { assert(fp); }
641653

642654
explicit error(const char* filename) {
643-
int fd = open(filename, O_APPEND | O_CREAT | O_RDWR, 0640);
655+
int fd = subprocess_open(filename, O_APPEND | O_CREAT | O_RDWR, 0640);
644656
if (fd == -1) throw OSError("File not found: ", errno);
645657
wr_ch_ = fd;
646658
}
@@ -803,28 +815,28 @@ class Streams
803815
void cleanup_fds()
804816
{
805817
if (write_to_child_ != -1 && read_from_parent_ != -1) {
806-
close(write_to_child_);
818+
subprocess_close(write_to_child_);
807819
}
808820
if (write_to_parent_ != -1 && read_from_child_ != -1) {
809-
close(read_from_child_);
821+
subprocess_close(read_from_child_);
810822
}
811823
if (err_write_ != -1 && err_read_ != -1) {
812-
close(err_read_);
824+
subprocess_close(err_read_);
813825
}
814826
}
815827

816828
void close_parent_fds()
817829
{
818-
if (write_to_child_ != -1) close(write_to_child_);
819-
if (read_from_child_ != -1) close(read_from_child_);
820-
if (err_read_ != -1) close(err_read_);
830+
if (write_to_child_ != -1) subprocess_close(write_to_child_);
831+
if (read_from_child_ != -1) subprocess_close(read_from_child_);
832+
if (err_read_ != -1) subprocess_close(err_read_);
821833
}
822834

823835
void close_child_fds()
824836
{
825-
if (write_to_parent_ != -1) close(write_to_parent_);
826-
if (read_from_parent_ != -1) close(read_from_parent_);
827-
if (err_write_ != -1) close(err_write_);
837+
if (write_to_parent_ != -1) subprocess_close(write_to_parent_);
838+
if (read_from_parent_ != -1) subprocess_close(read_from_parent_);
839+
if (err_write_ != -1) subprocess_close(err_write_);
828840
}
829841

830842
FILE* input() { return input_.get(); }
@@ -1161,8 +1173,8 @@ inline void Popen::execute_process() noexcept(false)
11611173
child_pid_ = fork();
11621174

11631175
if (child_pid_ < 0) {
1164-
close(err_rd_pipe);
1165-
close(err_wr_pipe);
1176+
subprocess_close(err_rd_pipe);
1177+
subprocess_close(err_wr_pipe);
11661178
throw OSError("fork failed", errno);
11671179
}
11681180

@@ -1172,14 +1184,14 @@ inline void Popen::execute_process() noexcept(false)
11721184
stream_.close_parent_fds();
11731185

11741186
//Close the read end of the error pipe
1175-
close(err_rd_pipe);
1187+
subprocess_close(err_rd_pipe);
11761188

11771189
detail::Child chld(this, err_wr_pipe);
11781190
chld.execute_child();
11791191
}
11801192
else
11811193
{
1182-
close (err_wr_pipe);// close child side of pipe, else get stuck in read below
1194+
subprocess_close(err_wr_pipe);// close child side of pipe, else get stuck in read below
11831195

11841196
stream_.close_child_fds();
11851197

@@ -1188,7 +1200,7 @@ inline void Popen::execute_process() noexcept(false)
11881200

11891201
FILE* err_fp = fdopen(err_rd_pipe, "r");
11901202
if (!err_fp) {
1191-
close(err_rd_pipe);
1203+
subprocess_close(err_rd_pipe);
11921204
throw OSError("fdopen failed", errno);
11931205
}
11941206
int read_bytes = util::read_atmost_n(err_fp, err_buf, SP_MAX_ERR_BUF_SIZ);
@@ -1278,13 +1290,13 @@ namespace detail {
12781290

12791291
// Close the duped descriptors
12801292
if (stream.read_from_parent_ != -1 && stream.read_from_parent_ > 2)
1281-
close(stream.read_from_parent_);
1293+
subprocess_close(stream.read_from_parent_);
12821294

12831295
if (stream.write_to_parent_ != -1 && stream.write_to_parent_ > 2)
1284-
close(stream.write_to_parent_);
1296+
subprocess_close(stream.write_to_parent_);
12851297

12861298
if (stream.err_write_ != -1 && stream.err_write_ > 2)
1287-
close(stream.err_write_);
1299+
subprocess_close(stream.err_write_);
12881300

12891301
// Replace the current image with the executable
12901302
sys_ret = execvp(parent_->exe_name_.c_str(), parent_->cargv_.data());
@@ -1311,15 +1323,15 @@ namespace detail {
13111323
#ifdef __USING_WINDOWS__
13121324
util::configure_pipe(&this->g_hChildStd_IN_Rd, &this->g_hChildStd_IN_Wr, &this->g_hChildStd_IN_Wr);
13131325
this->input(util::file_from_handle(this->g_hChildStd_IN_Wr, "w"));
1314-
this->write_to_child_ = _fileno(this->input());
1326+
this->write_to_child_ = subprocess_fileno(this->input());
13151327

13161328
util::configure_pipe(&this->g_hChildStd_OUT_Rd, &this->g_hChildStd_OUT_Wr, &this->g_hChildStd_OUT_Rd);
13171329
this->output(util::file_from_handle(this->g_hChildStd_OUT_Rd, "r"));
1318-
this->read_from_child_ = _fileno(this->output());
1330+
this->read_from_child_ = subprocess_fileno(this->output());
13191331

13201332
util::configure_pipe(&this->g_hChildStd_ERR_Rd, &this->g_hChildStd_ERR_Wr, &this->g_hChildStd_ERR_Rd);
13211333
this->error(util::file_from_handle(this->g_hChildStd_ERR_Rd, "r"));
1322-
this->err_read_ = _fileno(this->error());
1334+
this->err_read_ = subprocess_fileno(this->error());
13231335
#else
13241336

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

0 commit comments

Comments
 (0)