Abstract fd operations better in tool.
Windows and POSIX implement very similar fd operations, but differ
slightly:
- ssize_t in POSIX is usually int on Windows.
- POSIX needs EINTR retry loops.
- Windows wants _open rather than open, etc.
- POSIX fds and sockets are the same thing, while Windows sockets are
HANDLEs and leaves fd as a C runtime construct.
Rather than ad-hoc macros and redefinitions of ssize_t (which reportedly
upset MINGW), add some actual abstractions. While I'm here, add a scoped
file descriptor type.
That still leaves recv/send which are only used in one file, so defined
a socket_result_t for them.
Change-Id: I17fca2a50c77191f573852bfd27553996e3e9c3f
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/41725
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/tool/CMakeLists.txt b/tool/CMakeLists.txt
index 7f34017..7658713 100644
--- a/tool/CMakeLists.txt
+++ b/tool/CMakeLists.txt
@@ -8,6 +8,7 @@
client.cc
const.cc
digest.cc
+ fd.cc
file.cc
generate_ed25519.cc
genrsa.cc
diff --git a/tool/digest.cc b/tool/digest.cc
index 742fa7f..4adaf8d 100644
--- a/tool/digest.cc
+++ b/tool/digest.cc
@@ -37,7 +37,6 @@
OPENSSL_MSVC_PRAGMA(warning(pop))
#include <io.h>
#define PATH_MAX MAX_PATH
-typedef int ssize_t;
#endif
#include <openssl/digest.h>
@@ -45,19 +44,6 @@
#include "internal.h"
-struct close_delete {
- void operator()(int *fd) {
- BORINGSSL_CLOSE(*fd);
- }
-};
-
-template<typename T, typename R, R (*func) (T*)>
-struct func_delete {
- void operator()(T* obj) {
- func(obj);
- }
-};
-
// Source is an awkward expression of a union type in C++: Stdin | File filename.
struct Source {
enum Type {
@@ -79,37 +65,31 @@
static const char kStdinName[] = "standard input";
-// OpenFile opens the regular file named |filename| and sets |*out_fd| to be a
-// file descriptor to it. Returns true on sucess or prints an error to stderr
-// and returns false on error.
-static bool OpenFile(int *out_fd, const std::string &filename) {
- *out_fd = -1;
-
- int fd = BORINGSSL_OPEN(filename.c_str(), O_RDONLY | O_BINARY);
- if (fd < 0) {
+// OpenFile opens the regular file named |filename| and returns a file
+// descriptor to it.
+static ScopedFD OpenFile(const std::string &filename) {
+ ScopedFD fd = OpenFD(filename.c_str(), O_RDONLY | O_BINARY);
+ if (!fd) {
fprintf(stderr, "Failed to open input file '%s': %s\n", filename.c_str(),
strerror(errno));
- return false;
+ return ScopedFD();
}
- std::unique_ptr<int, close_delete> scoped_fd(&fd);
#if !defined(OPENSSL_WINDOWS)
struct stat st;
- if (fstat(fd, &st)) {
+ if (fstat(fd.get(), &st)) {
fprintf(stderr, "Failed to stat input file '%s': %s\n", filename.c_str(),
strerror(errno));
- return false;
+ return ScopedFD();
}
if (!S_ISREG(st.st_mode)) {
fprintf(stderr, "%s: not a regular file\n", filename.c_str());
- return false;
+ return ScopedFD();
}
#endif
- *out_fd = fd;
- scoped_fd.release();
- return true;
+ return fd;
}
// SumFile hashes the contents of |source| with |md| and sets |*out_hex| to the
@@ -119,16 +99,17 @@
// error.
static bool SumFile(std::string *out_hex, const EVP_MD *md,
const Source &source) {
- std::unique_ptr<int, close_delete> scoped_fd;
+ ScopedFD scoped_fd;
int fd;
if (source.is_stdin()) {
fd = 0;
} else {
- if (!OpenFile(&fd, source.filename())) {
+ scoped_fd = OpenFile(source.filename());
+ if (!scoped_fd) {
return false;
}
- scoped_fd.reset(&fd);
+ fd = scoped_fd.get();
}
static const size_t kBufSize = 8192;
@@ -141,21 +122,18 @@
}
for (;;) {
- ssize_t n;
-
- do {
- n = BORINGSSL_READ(fd, buf.get(), kBufSize);
- } while (n == -1 && errno == EINTR);
-
- if (n == 0) {
- break;
- } else if (n < 0) {
+ size_t n;
+ if (!ReadFromFD(fd, &n, buf.get(), kBufSize)) {
fprintf(stderr, "Failed to read from %s: %s\n",
source.is_stdin() ? kStdinName : source.filename().c_str(),
strerror(errno));
return false;
}
+ if (n == 0) {
+ break;
+ }
+
if (!EVP_DigestUpdate(ctx.get(), buf.get(), n)) {
fprintf(stderr, "Failed to update hash.\n");
return false;
@@ -221,25 +199,23 @@
// returns false.
static bool Check(const CheckModeArguments &args, const EVP_MD *md,
const Source &source) {
- std::unique_ptr<FILE, func_delete<FILE, int, fclose>> scoped_file;
FILE *file;
+ ScopedFILE scoped_file;
if (source.is_stdin()) {
file = stdin;
} else {
- int fd;
- if (!OpenFile(&fd, source.filename())) {
+ ScopedFD fd = OpenFile(source.filename());
+ if (!fd) {
return false;
}
- file = BORINGSSL_FDOPEN(fd, "rb");
- if (!file) {
+ scoped_file = FDToFILE(std::move(fd), "rb");
+ if (!scoped_file) {
perror("fdopen");
- BORINGSSL_CLOSE(fd);
return false;
}
-
- scoped_file = std::unique_ptr<FILE, func_delete<FILE, int, fclose>>(file);
+ file = scoped_file.get();
}
const size_t hex_size = EVP_MD_size(md) * 2;
diff --git a/tool/fd.cc b/tool/fd.cc
new file mode 100644
index 0000000..2c27ccd
--- /dev/null
+++ b/tool/fd.cc
@@ -0,0 +1,105 @@
+/* Copyright (c) 2020, Google Inc.
+ *
+ * Permission to use, copy, modify, and/or distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY
+ * SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION
+ * OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN
+ * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. */
+
+#include <openssl/base.h>
+
+#include <errno.h>
+#include <limits.h>
+#include <stdio.h>
+
+#include <algorithm>
+
+#include "internal.h"
+
+#if defined(OPENSSL_WINDOWS)
+#include <io.h>
+#else
+#include <fcntl.h>
+#include <unistd.h>
+#endif
+
+
+ScopedFD OpenFD(const char *path, int flags) {
+#if defined(OPENSSL_WINDOWS)
+ return ScopedFD(_open(path, flags));
+#else
+ int fd;
+ do {
+ fd = open(path, flags);
+ } while (fd == -1 && errno == EINTR);
+ return ScopedFD(fd);
+#endif
+}
+
+void CloseFD(int fd) {
+#if defined(OPENSSL_WINDOWS)
+ _close(fd);
+#else
+ close(fd);
+#endif
+}
+
+bool ReadFromFD(int fd, size_t *out_bytes_read, void *out, size_t num) {
+#if defined(OPENSSL_WINDOWS)
+ // On Windows, the buffer must be at most |INT_MAX|. See
+ // https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/read?view=vs-2019
+ int ret = _read(fd, out, std::min(size_t{INT_MAX}, num));
+#else
+ ssize_t ret;
+ do {
+ ret = read(fd, out, num);
+ } while (ret == -1 && errno == EINVAL);
+#endif
+
+ if (ret < 0) {
+ *out_bytes_read = 0;
+ return false;
+ }
+ *out_bytes_read = ret;
+ return true;
+}
+
+bool WriteToFD(int fd, size_t *out_bytes_written, const void *in, size_t num) {
+#if defined(OPENSSL_WINDOWS)
+ // The documentation for |_write| does not say the buffer must be at most
+ // |INT_MAX|, but clamp it to |INT_MAX| instead of |UINT_MAX| in case.
+ int ret = _write(fd, in, std::min(size_t{INT_MAX}, num));
+#else
+ ssize_t ret;
+ do {
+ ret = write(fd, in, num);
+ } while (ret == -1 && errno == EINVAL);
+#endif
+
+ if (ret < 0) {
+ *out_bytes_written = 0;
+ return false;
+ }
+ *out_bytes_written = ret;
+ return true;
+}
+
+ScopedFILE FDToFILE(ScopedFD fd, const char *mode) {
+ ScopedFILE ret;
+#if defined(OPENSSL_WINDOWS)
+ ret.reset(_fdopen(fd.get(), mode));
+#else
+ ret.reset(fdopen(fd.get(), mode));
+#endif
+ // |fdopen| takes ownership of |fd| on success.
+ if (ret) {
+ fd.release();
+ }
+ return ret;
+}
diff --git a/tool/internal.h b/tool/internal.h
index 4cace9d..eb9e4ba 100644
--- a/tool/internal.h
+++ b/tool/internal.h
@@ -18,32 +18,17 @@
#include <openssl/base.h>
#include <string>
+#include <utility>
#include <vector>
-OPENSSL_MSVC_PRAGMA(warning(push))
// MSVC issues warning C4702 for unreachable code in its xtree header when
// compiling with -D_HAS_EXCEPTIONS=0. See
// https://connect.microsoft.com/VisualStudio/feedback/details/809962
+OPENSSL_MSVC_PRAGMA(warning(push))
OPENSSL_MSVC_PRAGMA(warning(disable: 4702))
-
#include <map>
-
OPENSSL_MSVC_PRAGMA(warning(pop))
-#if defined(OPENSSL_WINDOWS)
- #define BORINGSSL_OPEN _open
- #define BORINGSSL_FDOPEN _fdopen
- #define BORINGSSL_CLOSE _close
- #define BORINGSSL_READ _read
- #define BORINGSSL_WRITE _write
-#else
- #define BORINGSSL_OPEN open
- #define BORINGSSL_FDOPEN fdopen
- #define BORINGSSL_CLOSE close
- #define BORINGSSL_READ read
- #define BORINGSSL_WRITE write
-#endif
-
struct FileCloser {
void operator()(FILE *file) {
fclose(file);
@@ -52,6 +37,67 @@
using ScopedFILE = std::unique_ptr<FILE, FileCloser>;
+// The following functions abstract between POSIX and Windows differences in
+// file descriptor I/O functions.
+
+// CloseFD behaves like |close|.
+void CloseFD(int fd);
+
+class ScopedFD {
+ public:
+ ScopedFD() {}
+ explicit ScopedFD(int fd) : fd_(fd) {}
+ ScopedFD(ScopedFD &&other) { *this = std::move(other); }
+ ScopedFD(const ScopedFD &) = delete;
+ ~ScopedFD() { reset(); }
+
+ ScopedFD &operator=(const ScopedFD &) = delete;
+ ScopedFD &operator=(ScopedFD &&other) {
+ reset();
+ fd_ = other.fd_;
+ other.fd_ = -1;
+ return *this;
+ }
+
+ explicit operator bool() const { return fd_ >= 0; }
+
+ int get() const { return fd_; }
+
+ void reset() {
+ if (fd_ >= 0) {
+ CloseFD(fd_);
+ }
+ fd_ = -1;
+ }
+
+ int release() {
+ int fd = fd_;
+ fd_ = -1;
+ return fd;
+ }
+
+ private:
+ int fd_ = -1;
+};
+
+// OpenFD behaves like |open| but handles |EINTR| and works on Windows.
+ScopedFD OpenFD(const char *path, int flags);
+
+// ReadFromFD reads up to |num| bytes from |fd| and writes the result to |out|.
+// On success, it returns true and sets |*out_bytes_read| to the number of bytes
+// read. Otherwise, it returns false and leaves an error in |errno|. On POSIX,
+// it handles |EINTR| internally.
+bool ReadFromFD(int fd, size_t *out_bytes_read, void *out, size_t num);
+
+// WriteToFD writes up to |num| bytes from |in| to |fd|. On success, it returns
+// true and sets |*out_bytes_written| to the number of bytes written. Otherwise,
+// it returns false and leaves an error in |errno|. On POSIX, it handles |EINTR|
+// internally.
+bool WriteToFD(int fd, size_t *out_bytes_written, const void *in, size_t num);
+
+// FDToFILE behaves like |fdopen|.
+ScopedFILE FDToFILE(ScopedFD fd, const char *mode);
+
enum ArgumentType {
kRequiredArgument,
kOptionalArgument,
diff --git a/tool/pkcs12.cc b/tool/pkcs12.cc
index a8ddb0e..3d8a1cd 100644
--- a/tool/pkcs12.cc
+++ b/tool/pkcs12.cc
@@ -40,12 +40,6 @@
#include "internal.h"
-#if defined(OPENSSL_WINDOWS)
-typedef int read_result_t;
-#else
-typedef ssize_t read_result_t;
-#endif
-
static const struct argument kArguments[] = {
{
"-dump", kOptionalArgument,
@@ -65,51 +59,52 @@
return false;
}
- int fd = BORINGSSL_OPEN(args_map["-dump"].c_str(), O_RDONLY);
- if (fd < 0) {
+ ScopedFD fd = OpenFD(args_map["-dump"].c_str(), O_RDONLY);
+ if (!fd) {
perror("open");
return false;
}
struct stat st;
- if (fstat(fd, &st)) {
+ if (fstat(fd.get(), &st)) {
perror("fstat");
- BORINGSSL_CLOSE(fd);
return false;
}
const size_t size = st.st_size;
std::unique_ptr<uint8_t[]> contents(new uint8_t[size]);
- read_result_t n;
size_t off = 0;
- do {
- n = BORINGSSL_READ(fd, &contents[off], size - off);
- if (n >= 0) {
- off += static_cast<size_t>(n);
+ while (off < size) {
+ size_t bytes_read;
+ if (!ReadFromFD(fd.get(), &bytes_read, contents.get() + off, size - off)) {
+ perror("read");
+ return false;
}
- } while ((n > 0 && off < size) || (n == -1 && errno == EINTR));
-
- if (off != size) {
- perror("read");
- BORINGSSL_CLOSE(fd);
- return false;
+ if (bytes_read == 0) {
+ fprintf(stderr, "Unexpected EOF\n");
+ return false;
+ }
+ off += bytes_read;
}
- BORINGSSL_CLOSE(fd);
-
printf("Enter password: ");
fflush(stdout);
char password[256];
off = 0;
- do {
- n = BORINGSSL_READ(0, &password[off], sizeof(password) - 1 - off);
- if (n >= 0) {
- off += static_cast<size_t>(n);
+ while (off < sizeof(password) - 1) {
+ size_t bytes_read;
+ if (!ReadFromFD(0, &bytes_read, password + off,
+ sizeof(password) - 1 - off)) {
+ perror("read");
+ return false;
}
- } while ((n > 0 && OPENSSL_memchr(password, '\n', off) == NULL &&
- off < sizeof(password) - 1) ||
- (n == -1 && errno == EINTR));
+
+ off += bytes_read;
+ if (bytes_read == 0 || OPENSSL_memchr(password, '\n', off) != nullptr) {
+ break;
+ }
+ }
char *newline = reinterpret_cast<char *>(OPENSSL_memchr(password, '\n', off));
if (newline == NULL) {
diff --git a/tool/transport_common.cc b/tool/transport_common.cc
index 7c5e962..88e9169 100644
--- a/tool/transport_common.cc
+++ b/tool/transport_common.cc
@@ -55,7 +55,6 @@
#include <ws2tcpip.h>
OPENSSL_MSVC_PRAGMA(warning(pop))
-typedef int ssize_t;
OPENSSL_MSVC_PRAGMA(comment(lib, "Ws2_32.lib"))
#endif
@@ -68,7 +67,10 @@
#include "transport_common.h"
-#if !defined(OPENSSL_WINDOWS)
+#if defined(OPENSSL_WINDOWS)
+using socket_result_t = int;
+#else
+using socket_result_t = ssize_t;
static int closesocket(int sock) {
return close(sock);
}
@@ -739,12 +741,13 @@
return true;
}
- ssize_t n;
- do {
- n = BORINGSSL_WRITE(1, buffer, ssl_ret);
- } while (n == -1 && errno == EINTR);
+ size_t n;
+ if (!WriteToFD(1, &n, buffer, ssl_ret)) {
+ fprintf(stderr, "Error writing to stdout.\n");
+ return false;
+ }
- if (n != ssl_ret) {
+ if (n != static_cast<size_t>(ssl_ret)) {
fprintf(stderr, "Short write to stderr.\n");
return false;
}
@@ -786,7 +789,7 @@
return false;
}
- ssize_t n;
+ socket_result_t n;
do {
n = recv(sock_, &buf_[buf_len_], sizeof(buf_) - buf_len_, 0);
} while (n == -1 && errno == EINTR);
@@ -871,7 +874,7 @@
size_t done = 0;
while (done < data_len) {
- ssize_t n;
+ socket_result_t n;
do {
n = send(sock, &data[done], data_len - done, 0);
} while (n == -1 && errno == EINTR);
