Allow rgbgfx - for stdin and stdout

Closes #1087

include/file.hpp

@@ -0,0 +1,95 @@
+/*
+ * This file is part of RGBDS.
+ *
+ * Copyright (c) 2022, Eldred Habert and RGBDS contributors.
+ *
+ * SPDX-License-Identifier: MIT
+ */
+
+#ifndef RGBDS_FILE_HPP
+#define RGBDS_FILE_HPP
+
+#include <array>
+#include <assert.h>
+#include <cassert>
+#include <fcntl.h>
+#include <filesystem>
+#include <fstream>
+#include <ios>
+#include <iostream>
+#include <streambuf>
+#include <string>
+#include <string.h>
+#include <string_view>
+#include <variant>
+
+#include "helpers.h"
+#include "platform.h"
+
+#include "gfx/main.hpp"
+
+// Convenience feature for visiting the below.
+template<typename... Ts>
+struct Visitor : Ts... {
+	using Ts::operator()...;
+};
+template<typename... Ts>
+Visitor(Ts...) -> Visitor<Ts...>;
+
+class File {
+	// Construct a `std::streambuf *` by default, since it's probably lighter than a `filebuf`.
+	std::variant<std::streambuf *, std::filebuf> _file;
+
+public:
+	File() {}
+	~File() { close(); }
+
+	/**
+	 * This should only be called once, and before doing any `->` operations.
+	 * Returns `nullptr` on error, and a non-null pointer otherwise.
+	 */
+	File *open(std::filesystem::path const &path, std::ios_base::openmode mode) {
+		if (path != "-") {
+			return _file.emplace<std::filebuf>().open(path, mode) ? this : nullptr;
+		} else if (mode & std::ios_base::in) {
+			assert(!(mode & std::ios_base::out));
+			_file.emplace<std::streambuf *>(std::cin.rdbuf());
+			if (setmode(STDIN_FILENO, mode & std::ios_base::binary ? O_BINARY : O_TEXT) == -1) {
+				fatal("Failed to set stdin to %s mode: %s",
+				      mode & std::ios_base::binary ? "binary" : "text", strerror(errno));
+			}
+		} else {
+			assert(mode & std::ios_base::out);
+			_file.emplace<std::streambuf *>(std::cout.rdbuf());
+		}
+		return this;
+	}
+	std::streambuf &operator*() {
+		return std::visit(Visitor{[](std::filebuf &file) -> std::streambuf & { return file; },
+		                          [](std::streambuf *buf) -> std::streambuf & { return *buf; }},
+		                  _file);
+	}
+	std::streambuf const &operator*() const {
+		// The non-`const` version does not perform any modifications, so it's okay.
+		return **const_cast<File *>(this);
+	}
+	std::streambuf *operator->() { return &**this; }
+	std::streambuf const *operator->() const {
+		// See the `operator*` equivalent.
+		return const_cast<File *>(this)->operator->();
+	}
+	File *close() {
+		return std::visit(Visitor{[this](std::filebuf &file) {
+			                          // This is called by the destructor, and an explicit `close`
+			                          // shouldn't close twice.
+			                          _file.emplace<std::streambuf *>(nullptr);
+			                          return file.close() != nullptr;
+		                          },
+		                          [](std::streambuf *buf) { return buf != nullptr; }},
+		                  _file)
+		           ? this
+		           : nullptr;
+	}
+};
+
+#endif // RGBDS_FILE_HPP

include/platform.h

@@ -63,8 +63,10 @@
 # define O_RDWR _O_RDWR
 # define S_ISREG(field) ((field) & _S_IFREG)
 # define O_BINARY _O_BINARY
+# define O_TEXT _O_TEXT
 #elif !defined(O_BINARY) // Cross-compilers define O_BINARY
 # define O_BINARY 0 // POSIX says we shouldn't care!
+# define O_TEXT 0 // Assume that it's not defined either
 #endif // _MSC_VER
 
 // Windows has stdin and stdout open as text by default, which we may not want
@@ -72,7 +74,7 @@
 # include <io.h>
 # define setmode(fd, mode) _setmode(fd, mode)
 #else
-# define setmode(fd, mode) ((void)0)
+# define setmode(fd, mode) (0)
 #endif
 
 #endif // RGBDS_PLATFORM_H

man/rgbgfx.1

@@ -72,6 +72,18 @@ All of these are equivalent:
 .Ql 0X2A ,
 .Ql 0x2a .
 .Pp
+Unless otherwise noted, passing
+.Ql -
+(a single dash) as a file name makes
+.Nm
+use standard input (for input files) or standard output (for output files).
+To suppress this behavior, and open a file in the current directory actually called
+.Ql - ,
+pass
+.Ql ./-
+instead.
+Using standard input or output more than once in a single command will likely produce unexpected results.
+.Pp
 The following options are accepted:
 .Bl -tag -width Ds
 .It Fl a Ar attrmap , Fl Fl attr-map Ar attrmap
@@ -145,7 +157,9 @@ The expected format is
 .Ql format:path ,
 where
 .Ar path
-is a path to a file, which will be processed according to the
+is a path to a file
+.Ql ( -
+is not treated specially), which will be processed according to the
 .Ar format .
 See
 .Sx PALETTE SPECIFICATION FORMATS

src/fix/main.c

@@ -1174,8 +1174,8 @@ static bool processFilename(char const *name)
 {
 	nbErrors = 0;
 	if (!strcmp(name, "-")) {
-		setmode(STDIN_FILENO, O_BINARY);
+		(void)setmode(STDIN_FILENO, O_BINARY);
-		setmode(STDOUT_FILENO, O_BINARY);
+		(void)setmode(STDOUT_FILENO, O_BINARY);
 		name = "<stdin>";
 		processFile(STDIN_FILENO, STDOUT_FILENO, name, 0);
 

src/gfx/main.cpp

@@ -23,8 +23,10 @@
 #include <stdlib.h>
 #include <string.h>
 #include <string_view>
+#include <type_traits>
 
 #include "extern/getopt.h"
+#include "file.hpp"
 #include "platform.h"
 #include "version.h"
 
@@ -253,12 +255,13 @@ static void registerInput(char const *arg) {
  * @param argPool Argument characters will be appended to this vector, for storage purposes.
  */
 static std::vector<size_t> readAtFile(std::string const &path, std::vector<char> &argPool) {
-	std::filebuf file;
+	File file;
 	if (!file.open(path, std::ios_base::in)) {
 		fatal("Error reading @%s: %s", path.c_str(), strerror(errno));
 	}
 
-	static_assert(decltype(file)::traits_type::eof() == EOF,
+	// We only filter out `EOF`, but calling `isblank()` on anything else is UB!
+	static_assert(std::remove_reference_t<decltype(*file)>::traits_type::eof() == EOF,
 	              "isblank(char_traits<...>::eof()) is UB!");
 	std::vector<size_t> argvOfs;
 
@@ -267,7 +270,7 @@ static std::vector<size_t> readAtFile(std::string const &path, std::vector<char>
 
 		// First, discard any leading whitespace
 		do {
-			c = file.sbumpc();
+			c = file->sbumpc();
 			if (c == EOF) {
 				return argvOfs;
 			}
@@ -275,7 +278,7 @@ static std::vector<size_t> readAtFile(std::string const &path, std::vector<char>
 
 		switch (c) {
 		case '#': // If it's a comment, discard everything until EOL
-			while ((c = file.sbumpc()) != '\n') {
+			while ((c = file->sbumpc()) != '\n') {
 				if (c == EOF) {
 					return argvOfs;
 				}
@@ -283,7 +286,7 @@ static std::vector<size_t> readAtFile(std::string const &path, std::vector<char>
 			continue; // Start processing the next line
 		// If it's an empty line, ignore it
 		case '\r': // Assuming CRLF here
-			file.sbumpc(); // Discard the upcoming '\n'
+			file->sbumpc(); // Discard the upcoming '\n'
 			[[fallthrough]];
 		case '\n':
 			continue; // Start processing the next line
@@ -298,11 +301,11 @@ static std::vector<size_t> readAtFile(std::string const &path, std::vector<char>
 			// on `vector` and `sbumpc` to do the right thing here.
 			argPool.push_back(c); // Push the character we've already read
 			for (;;) {
-				c = file.sbumpc();
+				c = file->sbumpc();
-				if (isblank(c) || c == '\n' || c == EOF) {
+				if (c == EOF || c == '\n' || isblank(c)) {
 					break;
 				} else if (c == '\r') {
-					file.sbumpc(); // Discard the '\n'
+					file->sbumpc(); // Discard the '\n'
 					break;
 				}
 				argPool.push_back(c);
@@ -311,10 +314,10 @@ static std::vector<size_t> readAtFile(std::string const &path, std::vector<char>
 
 			// Discard whitespace until the next argument (candidate)
 			while (isblank(c)) {
-				c = file.sbumpc();
+				c = file->sbumpc();
 			}
 			if (c == '\r') {
-				c = file.sbumpc(); // Skip the '\n'
+				c = file->sbumpc(); // Skip the '\n'
 			}
 		} while (c != '\n' && c != EOF); // End if we reached EOL
 	}

src/gfx/process.cpp

@@ -27,6 +27,7 @@
 #include <vector>
 
 #include "defaultinitalloc.hpp"
+#include "file.hpp"
 #include "helpers.h"
 #include "itertools.hpp"
 
@@ -77,7 +78,7 @@ public:
 
 class Png {
 	std::string const &path;
-	std::filebuf file{};
+	File file{};
 	png_structp png = nullptr;
 	png_infop info = nullptr;
 
@@ -105,13 +106,14 @@ class Png {
 	static void readData(png_structp png, png_bytep data, size_t length) {
 		Png *self = reinterpret_cast<Png *>(png_get_io_ptr(png));
 		std::streamsize expectedLen = length;
-		std::streamsize nbBytesRead = self->file.sgetn(reinterpret_cast<char *>(data), expectedLen);
+		std::streamsize nbBytesRead =
+		    self->file->sgetn(reinterpret_cast<char *>(data), expectedLen);
 
 		if (nbBytesRead != expectedLen) {
 			fatal("Error reading input image (\"%s\"): file too short (expected at least %zd more "
 			      "bytes after reading %lld)",
 			      self->path.c_str(), length - nbBytesRead,
-			      self->file.pubseekoff(0, std::ios_base::cur));
+			      self->file->pubseekoff(0, std::ios_base::cur));
 		}
 	}
 
@@ -182,7 +184,7 @@ public:
 
 		std::array<unsigned char, 8> pngHeader;
 
-		if (file.sgetn(reinterpret_cast<char *>(pngHeader.data()), pngHeader.size())
+		if (file->sgetn(reinterpret_cast<char *>(pngHeader.data()), pngHeader.size())
 		        != static_cast<std::streamsize>(pngHeader.size()) // Not enough bytes?
 		    || png_sig_cmp(pngHeader.data(), 0, pngHeader.size()) != 0) {
 			fatal("Input file (\"%s\") is not a PNG image!", path.c_str());
@@ -624,7 +626,7 @@ static std::tuple<DefaultInitVec<size_t>, std::vector<Palette>>
 }
 
 static void outputPalettes(std::vector<Palette> const &palettes) {
-	std::filebuf output;
+	File output;
 	if (!output.open(options.palettes, std::ios_base::out | std::ios_base::binary)) {
 		fatal("Failed to open \"%s\": %s", options.palettes.c_str(), strerror(errno));
 	}
@@ -632,8 +634,8 @@ static void outputPalettes(std::vector<Palette> const &palettes) {
 	for (Palette const &palette : palettes) {
 		for (uint8_t i = 0; i < options.nbColorsPerPal; ++i) {
 			uint16_t color = palette.colors[i]; // Will return `UINT16_MAX` for unused slots
-			output.sputc(color & 0xFF);
+			output->sputc(color & 0xFF);
-			output.sputc(color >> 8);
+			output->sputc(color >> 8);
 		}
 	}
 }
@@ -752,7 +754,7 @@ namespace unoptimized {
 static void outputTileData(Png const &png, DefaultInitVec<AttrmapEntry> const &attrmap,
                            std::vector<Palette> const &palettes,
                            DefaultInitVec<size_t> const &mappings) {
-	std::filebuf output;
+	File output;
 	if (!output.open(options.output, std::ios_base::out | std::ios_base::binary)) {
 		fatal("Failed to open \"%s\": %s", options.output.c_str(), strerror(errno));
 	}
@@ -768,9 +770,9 @@ static void outputTileData(Png const &png, DefaultInitVec<AttrmapEntry> const &a
 		Palette const &palette = palettes[attr.getPalID(mappings)];
 		for (uint32_t y = 0; y < 8; ++y) {
 			uint16_t bitplanes = TileData::rowBitplanes(tile, palette, y);
-			output.sputc(bitplanes & 0xFF);
+			output->sputc(bitplanes & 0xFF);
 			if (options.bitDepth == 2) {
-				output.sputc(bitplanes >> 8);
+				output->sputc(bitplanes >> 8);
 			}
 		}
 
@@ -784,7 +786,7 @@ static void outputTileData(Png const &png, DefaultInitVec<AttrmapEntry> const &a
 
 static void outputMaps(DefaultInitVec<AttrmapEntry> const &attrmap,
                        DefaultInitVec<size_t> const &mappings) {
-	std::optional<std::filebuf> tilemapOutput, attrmapOutput, palmapOutput;
+	std::optional<File> tilemapOutput, attrmapOutput, palmapOutput;
 	if (!options.tilemap.empty()) {
 		tilemapOutput.emplace();
 		if (!tilemapOutput->open(options.tilemap, std::ios_base::out | std::ios_base::binary)) {
@@ -814,14 +816,14 @@ static void outputMaps(DefaultInitVec<AttrmapEntry> const &attrmap,
 		}
 
 		if (tilemapOutput.has_value()) {
-			tilemapOutput->sputc(tileID + options.baseTileIDs[bank]);
+			(*tilemapOutput)->sputc(tileID + options.baseTileIDs[bank]);
 		}
 		if (attrmapOutput.has_value()) {
 			uint8_t palID = attr.getPalID(mappings) & 7;
-			attrmapOutput->sputc(palID | bank << 3); // The other flags are all 0
+			(*attrmapOutput)->sputc(palID | bank << 3); // The other flags are all 0
 		}
 		if (palmapOutput.has_value()) {
-			palmapOutput->sputc(attr.getPalID(mappings));
+			(*palmapOutput)->sputc(attr.getPalID(mappings));
 		}
 		++tileID;
 	}
@@ -896,7 +898,7 @@ static UniqueTiles dedupTiles(Png const &png, DefaultInitVec<AttrmapEntry> &attr
 }
 
 static void outputTileData(UniqueTiles const &tiles) {
-	std::filebuf output;
+	File output;
 	if (!output.open(options.output, std::ios_base::out | std::ios_base::binary)) {
 		fatal("Failed to create \"%s\": %s", options.output.c_str(), strerror(errno));
 	}
@@ -906,24 +908,24 @@ static void outputTileData(UniqueTiles const &tiles) {
 		TileData const *tile = *iter;
 		assert(tile->tileID == tileID);
 		++tileID;
-		output.sputn(reinterpret_cast<char const *>(tile->data().data()), options.bitDepth * 8);
+		output->sputn(reinterpret_cast<char const *>(tile->data().data()), options.bitDepth * 8);
 	}
 }
 
 static void outputTilemap(DefaultInitVec<AttrmapEntry> const &attrmap) {
-	std::filebuf output;
+	File output;
 	if (!output.open(options.tilemap, std::ios_base::out | std::ios_base::binary)) {
 		fatal("Failed to create \"%s\": %s", options.tilemap.c_str(), strerror(errno));
 	}
 
 	for (AttrmapEntry const &entry : attrmap) {
-		output.sputc(entry.tileID); // The tile ID has already been converted
+		output->sputc(entry.tileID); // The tile ID has already been converted
 	}
 }
 
 static void outputAttrmap(DefaultInitVec<AttrmapEntry> const &attrmap,
                           DefaultInitVec<size_t> const &mappings) {
-	std::filebuf output;
+	File output;
 	if (!output.open(options.attrmap, std::ios_base::out | std::ios_base::binary)) {
 		fatal("Failed to create \"%s\": %s", options.attrmap.c_str(), strerror(errno));
 	}
@@ -932,19 +934,19 @@ static void outputAttrmap(DefaultInitVec<AttrmapEntry> const &attrmap,
 		uint8_t attr = entry.xFlip << 5 | entry.yFlip << 6;
 		attr |= entry.bank << 3;
 		attr |= entry.getPalID(mappings) & 7;
-		output.sputc(attr);
+		output->sputc(attr);
 	}
 }
 
 static void outputPalmap(DefaultInitVec<AttrmapEntry> const &attrmap,
                          DefaultInitVec<size_t> const &mappings) {
-	std::filebuf output;
+	File output;
 	if (!output.open(options.attrmap, std::ios_base::out | std::ios_base::binary)) {
 		fatal("Failed to create \"%s\": %s", options.attrmap.c_str(), strerror(errno));
 	}
 
 	for (AttrmapEntry const &entry : attrmap) {
-		output.sputc(entry.getPalID(mappings));
+		output->sputc(entry.getPalID(mappings));
 	}
 }
 

src/gfx/reverse.cpp

@@ -21,13 +21,14 @@
 #include <vector>
 
 #include "defaultinitalloc.hpp"
+#include "file.hpp"
 #include "helpers.h"
 #include "itertools.hpp"
 
 #include "gfx/main.hpp"
 
 static DefaultInitVec<uint8_t> readInto(std::string path) {
-	std::filebuf file;
+	File file;
 	if (!file.open(path, std::ios::in | std::ios::binary)) {
 		fatal("Failed to open \"%s\": %s", path.c_str(), strerror(errno));
 	}
@@ -40,7 +41,7 @@ static DefaultInitVec<uint8_t> readInto(std::string path) {
 
 		// Fill the new area ([oldSize; curSize[) with bytes
 		size_t nbRead =
-		    file.sgetn(reinterpret_cast<char *>(&data.data()[oldSize]), curSize - oldSize);
+		    file->sgetn(reinterpret_cast<char *>(&data.data()[oldSize]), curSize - oldSize);
 		if (nbRead != curSize - oldSize) {
 			// Shrink the vector to discard bytes that weren't read
 			data.resize(oldSize + nbRead);
@@ -68,13 +69,13 @@ static void pngWarning(png_structp png, char const *msg) {
 }
 
 void writePng(png_structp png, png_bytep data, size_t length) {
-	auto &pngFile = *static_cast<std::filebuf *>(png_get_io_ptr(png));
+	auto &pngFile = *static_cast<File *>(png_get_io_ptr(png));
-	pngFile.sputn(reinterpret_cast<char *>(data), length);
+	pngFile->sputn(reinterpret_cast<char *>(data), length);
 }
 
 void flushPng(png_structp png) {
-	auto &pngFile = *static_cast<std::filebuf *>(png_get_io_ptr(png));
+	auto &pngFile = *static_cast<File *>(png_get_io_ptr(png));
-	pngFile.pubsync();
+	pngFile->pubsync();
 }
 
 void reverse() {
@@ -146,7 +147,7 @@ void reverse() {
 	    {Rgba(0xffffffff), Rgba(0xaaaaaaff), Rgba(0x555555ff), Rgba(0x000000ff)}
     };
 	if (!options.palettes.empty()) {
-		std::filebuf file;
+		File file;
 		if (!file.open(options.palettes, std::ios::in | std::ios::binary)) {
 			fatal("Failed to open \"%s\": %s", options.palettes.c_str(), strerror(errno));
 		}
@@ -155,7 +156,7 @@ void reverse() {
 		std::array<uint8_t, sizeof(uint16_t) * 4> buf; // 4 colors
 		size_t nbRead;
 		do {
-			nbRead = file.sgetn(reinterpret_cast<char *>(buf.data()), buf.size());
+			nbRead = file->sgetn(reinterpret_cast<char *>(buf.data()), buf.size());
 			if (nbRead == buf.size()) {
 				// Expand the colors
 				auto &palette = palettes.emplace_back();
@@ -233,7 +234,7 @@ void reverse() {
 	}
 
 	options.verbosePrint(Options::VERB_LOG_ACT, "Writing image...\n");
-	std::filebuf pngFile;
+	File pngFile;
 	if (!pngFile.open(options.input, std::ios::out | std::ios::binary)) {
 		fatal("Failed to create \"%s\": %s", options.input.c_str(), strerror(errno));
 	}

test/gfx/test.sh

@@ -12,6 +12,8 @@ red="$(tput setaf 1)"
 green="$(tput setaf 2)"
 rescolors="$(tput op)"
 
+RGBGFX=../../rgbgfx
+
 rc=0
 new_test() {
 	cmdline="${*@Q}"
@@ -42,7 +44,7 @@ rm -f out*.png result.png
 
 for f in *.png; do
 	flags="$([[ -e "${f%.png}.flags" ]] && echo "@${f%.png}.flags")"
-	new_test ../../rgbgfx $flags "$f"
+	new_test "$RGBGFX" $flags "$f"
 
 	if [[ -e "${f%.png}.err" ]]; then
 		test 2>"$errtmp"