From 45093a336a099bae5b9773de4d68d9502d554fc0 Mon Sep 17 00:00:00 2001 From: Marc Emery Date: Tue, 14 Apr 2026 21:36:30 +0200 Subject: [PATCH] Improve file open error messages Define open_(o|i)fstream_and_log_error in log.h to: - quote 'filename' - add error cause to easier troubleshoot - use existing consistent string style - easily allows OS specific message Introduce it when file are opened and add it where error message was missing. --- common/kernel/command.cc | 21 +++++++++------------ common/kernel/log.cc | 23 +++++++++++++++++++++++ common/kernel/log.h | 10 +++++++++- common/kernel/pybindings.cc | 4 +--- common/kernel/svg.cc | 2 +- common/place/static_util.h | 5 +++++ common/route/router2.cc | 13 +++---------- ecp5/bitstream.cc | 7 ++----- ecp5/main.cc | 6 ++---- generic/viaduct/fabulous/fabulous.cc | 5 +---- generic/viaduct/fabulous/pcf.cc | 5 +---- himbaechel/uarch/gatemate/bitstream.cc | 4 +--- himbaechel/uarch/gatemate/ccf.cc | 4 +--- himbaechel/uarch/gowin/gowin.cc | 6 ++---- himbaechel/uarch/ng-ultra/bitstream.cc | 4 +--- himbaechel/uarch/ng-ultra/csv.cc | 4 +--- himbaechel/uarch/xilinx/fasm.cc | 7 ++----- himbaechel/uarch/xilinx/xdc.cc | 4 +--- ice40/arch_pybindings.cc | 5 ++--- ice40/main.cc | 6 +++--- machxo2/bitstream.cc | 2 +- machxo2/main.cc | 4 +--- mistral/main.cc | 11 ++++++----- nexus/main.cc | 8 ++------ 24 files changed, 81 insertions(+), 89 deletions(-) diff --git a/common/kernel/command.cc b/common/kernel/command.cc index 5b0f4695..d78dd944 100644 --- a/common/kernel/command.cc +++ b/common/kernel/command.cc @@ -609,13 +609,13 @@ int CommandHandler::executeMain(std::unique_ptr ctx) try { if (vm.count("json")) { std::string filename = vm["json"].as(); - std::ifstream f(filename); + auto f = open_ifstream_and_log_error(filename, "JSON file"); if (!parse_json(f, filename, w.getContext())) log_error("Loading design failed.\n"); if (vm.count("sdc")) { std::string sdc_filename = vm["sdc"].as(); - std::ifstream sdc_stream(sdc_filename); + auto sdc_stream = open_ifstream_and_log_error(sdc_filename, "SDC file"); w.getContext()->read_sdc(sdc_stream); } @@ -635,13 +635,14 @@ int CommandHandler::executeMain(std::unique_ptr ctx) #endif if (vm.count("json")) { std::string filename = vm["json"].as(); - std::ifstream f(filename); + auto f = open_ifstream_and_log_error(filename, "'--json' file"); + if (!parse_json(f, filename, ctx.get())) log_error("Loading design failed.\n"); if (vm.count("sdc")) { std::string sdc_filename = vm["sdc"].as(); - std::ifstream sdc_stream(sdc_filename); + auto sdc_stream = open_ifstream_and_log_error(sdc_filename, "SDC file"); ctx->read_sdc(sdc_stream); } @@ -703,24 +704,20 @@ int CommandHandler::executeMain(std::unique_ptr ctx) if (vm.count("write")) { std::string filename = vm["write"].as(); - std::ofstream f(filename); + auto f = open_ofstream_and_log_error(filename, "JSON '--write' file"); if (!write_json_file(f, filename, ctx.get())) log_error("Saving design failed.\n"); } if (vm.count("sdf")) { std::string filename = vm["sdf"].as(); - std::ofstream f(filename); - if (!f) - log_error("Failed to open SDF file '%s' for writing.\n", filename.c_str()); + auto f = open_ofstream_and_log_error(filename, "SDF file"); ctx->writeSDF(f, vm.count("sdf-cvc")); } if (vm.count("report")) { std::string filename = vm["report"].as(); - std::ofstream f(filename); - if (!f) - log_error("Failed to open report file '%s' for writing.\n", filename.c_str()); + auto f = open_ofstream_and_log_error(filename, "report file"); ctx->writeJsonReport(f); } @@ -778,7 +775,7 @@ void CommandHandler::load_json(Context *ctx, std::string filename) setupContext(ctx); setupArchContext(ctx); { - std::ifstream f(filename); + auto f = open_ifstream_and_log_error(filename, "JSON file"); if (!parse_json(f, filename, ctx)) log_error("Loading design failed.\n"); } diff --git a/common/kernel/log.cc b/common/kernel/log.cc index a0bdd64e..af8901cf 100644 --- a/common/kernel/log.cc +++ b/common/kernel/log.cc @@ -17,6 +17,7 @@ * */ +#include #include #include #include @@ -187,6 +188,28 @@ void log_error(const char *format, ...) logv_error(format, ap); } +std::ifstream open_ifstream_and_log_error(std::string filename, const char *file_description) +{ + std::ifstream file(filename); + if (!file.is_open()) { + log_error("Failed to open %s '%s': %s.\n", file_description, filename.c_str(), + std::error_code(errno, std::generic_category()).message().c_str()); + } + + return file; +} + +std::ofstream open_ofstream_and_log_error(std::string filename, const char *file_description) +{ + std::ofstream file(filename); + if (!file.is_open()) { + log_error("Failed to open %s '%s' for writing: %s.\n", file_description, filename.c_str(), + std::error_code(errno, std::generic_category()).message().c_str()); + } + + return file; +} + void log_break() { if (log_newline_count < 2) diff --git a/common/kernel/log.h b/common/kernel/log.h index cba539f4..e5bdf6ae 100644 --- a/common/kernel/log.h +++ b/common/kernel/log.h @@ -20,8 +20,8 @@ #ifndef LOG_H #define LOG_H +#include #include -#include #include #include #include @@ -30,6 +30,8 @@ #include "hashlib.h" #include "nextpnr_namespaces.h" +#include + NEXTPNR_NAMESPACE_BEGIN typedef std::function log_write_type; @@ -88,6 +90,12 @@ static inline void log_assert_worker(bool cond, const char *expr, const char *fi #define log_abort() log_error("Abort in %s:%d.\n", __FILE__, __LINE__) +/// open `filename`, if error log "Failed to open {file_description} '{filename}'" with cause +std::ifstream open_ifstream_and_log_error(std::string filename, const char *file_description); + +/// open `filename`, if error log "Failed to open {file_description} '{filename}' for writing" with cause +std::ofstream open_ofstream_and_log_error(std::string filename, const char *file_description); + NEXTPNR_NAMESPACE_END #endif diff --git a/common/kernel/pybindings.cc b/common/kernel/pybindings.cc index 59cd02b5..c1791fb4 100644 --- a/common/kernel/pybindings.cc +++ b/common/kernel/pybindings.cc @@ -50,9 +50,7 @@ bool operator==(const PortRef &a, const PortRef &b) { return (a.cell == b.cell) // Load a JSON file into a design void parse_json_shim(std::string filename, Context &d) { - std::ifstream inf(filename); - if (!inf) - throw std::runtime_error("failed to open file " + filename); + auto inf = open_ifstream_and_log_error(filename, "JSON file"); parse_json(inf, filename, &d); } diff --git a/common/kernel/svg.cc b/common/kernel/svg.cc index 4fef53b7..45df46b1 100644 --- a/common/kernel/svg.cc +++ b/common/kernel/svg.cc @@ -145,7 +145,7 @@ struct SVGWriter void Context::writeSVG(const std::string &filename, const std::string &flags) const { - std::ofstream out(filename); + auto out = open_ofstream_and_log_error(filename, "SVG file"); SVGWriter(this, out)(flags); } diff --git a/common/place/static_util.h b/common/place/static_util.h index bbaa3d9b..ece951b1 100644 --- a/common/place/static_util.h +++ b/common/place/static_util.h @@ -21,6 +21,7 @@ #define STATIC_UTIL_H #include +#include "log.h" #include "nextpnr_assertions.h" #include "nextpnr_namespaces.h" @@ -105,6 +106,10 @@ struct FFTArray void write_csv(const std::string &filename) const { std::ofstream out(filename); + if (!out.is_open()) { + log_error("Failed to open CSV file for writing '%s': %s.\n", filename.c_str(), + std::error_code(errno, std::generic_category()).message().c_str()); + } NPNR_ASSERT(out); for (int y = 0; y < m_height; y++) { for (int x = 0; x < m_width; x++) { diff --git a/common/route/router2.cc b/common/route/router2.cc index 5a737b13..334f3357 100644 --- a/common/route/router2.cc +++ b/common/route/router2.cc @@ -1718,26 +1718,19 @@ struct Router2 if (!cfg.heatmap.empty()) { { std::string filename(cfg.heatmap + "_congestion_by_wiretype_" + std::to_string(iter) + ".csv"); - std::ofstream cong_map(filename); - if (!cong_map) - log_error("Failed to open congestion-by-wiretype heatmap %s for writing.\n", filename.c_str()); + auto cong_map = open_ofstream_and_log_error(filename, "congestion-by-wiretype heatmap"); write_congestion_by_wiretype_heatmap(cong_map); log_info(" wrote congestion-by-wiretype heatmap to %s.\n", filename.c_str()); } { std::string filename(cfg.heatmap + "_utilisation_by_wiretype_" + std::to_string(iter) + ".csv"); - std::ofstream cong_map(filename); - if (!cong_map) - log_error("Failed to open utilisation-by-wiretype heatmap %s for writing.\n", filename.c_str()); + auto cong_map = open_ofstream_and_log_error(filename, "utilisation-by-wiretype heatmap"); write_utilisation_by_wiretype_heatmap(cong_map); log_info(" wrote utilisation-by-wiretype heatmap to %s.\n", filename.c_str()); } { std::string filename(cfg.heatmap + "_congestion_by_coordinate_" + std::to_string(iter) + ".csv"); - std::ofstream cong_map(filename); - if (!cong_map) - log_error("Failed to open congestion-by-coordinate heatmap %s for writing.\n", - filename.c_str()); + auto cong_map = open_ofstream_and_log_error(filename, "congestion-by-coordinate heatmap"); write_congestion_by_coordinate_heatmap(cong_map); log_info(" wrote congestion-by-coordinate heatmap to %s.\n", filename.c_str()); } diff --git a/ecp5/bitstream.cc b/ecp5/bitstream.cc index 31b5041d..2648eff9 100644 --- a/ecp5/bitstream.cc +++ b/ecp5/bitstream.cc @@ -1364,10 +1364,7 @@ struct ECP5Bitgen void run(const std::string &base_config_file) { if (!base_config_file.empty()) { - std::ifstream config_file(base_config_file); - if (!config_file) { - log_error("failed to open base config file '%s'\n", base_config_file.c_str()); - } + auto config_file = open_ifstream_and_log_error(base_config_file, "base config file"); config_file >> cc; } else { switch (ctx->args.type) { @@ -1599,7 +1596,7 @@ void write_bitstream(Context *ctx, std::string base_config_file, std::string tex // Configure chip if (!text_config_file.empty()) { - std::ofstream out_config(text_config_file); + auto out_config = open_ofstream_and_log_error(text_config_file, "text config file"); out_config << bitgen.cc; } } diff --git a/ecp5/main.cc b/ecp5/main.cc index 9aa92c1e..c08cebb1 100644 --- a/ecp5/main.cc +++ b/ecp5/main.cc @@ -266,11 +266,9 @@ void ECP5CommandHandler::customAfterLoad(Context *ctx) if (vm.count("lpf")) { std::vector files = vm["lpf"].as>(); for (const auto &filename : files) { - std::ifstream in(filename); - if (!in) - log_error("failed to open LPF file '%s'\n", filename.c_str()); + auto in = open_ifstream_and_log_error(filename, "LPF file"); if (!ctx->apply_lpf(filename, in)) - log_error("failed to parse LPF file '%s'\n", filename.c_str()); + log_error("Failed to parse LPF file '%s'\n", filename.c_str()); } for (auto &cell : ctx->cells) { diff --git a/generic/viaduct/fabulous/fabulous.cc b/generic/viaduct/fabulous/fabulous.cc index e3b9bc65..aec8a196 100644 --- a/generic/viaduct/fabulous/fabulous.cc +++ b/generic/viaduct/fabulous/fabulous.cc @@ -213,10 +213,7 @@ struct FabulousImpl : ViaductAPI std::ifstream open_data_rel(const std::string &postfix) { const std::string filename(fab_root + postfix); - std::ifstream in(filename); - if (!in) - log_error("failed to open data file '%s' (is FAB_ROOT set correctly?)\n", filename.c_str()); - return in; + return open_ifstream_and_log_error(filename, "data file (is FAB_ROOT set correctly?)"); } std::string fab_root; diff --git a/generic/viaduct/fabulous/pcf.cc b/generic/viaduct/fabulous/pcf.cc index 011750e8..89964cb9 100644 --- a/generic/viaduct/fabulous/pcf.cc +++ b/generic/viaduct/fabulous/pcf.cc @@ -537,10 +537,7 @@ struct FABulousDesignConstraints void apply_constraints() { - std::ifstream in(filename); - if (!in) { - log_error("failed to open constraint file\n"); - } + auto in = open_ifstream_and_log_error(filename, "constraint file"); std::string line; std::string accumulated_line; diff --git a/himbaechel/uarch/gatemate/bitstream.cc b/himbaechel/uarch/gatemate/bitstream.cc index 30b78d61..428d20ac 100644 --- a/himbaechel/uarch/gatemate/bitstream.cc +++ b/himbaechel/uarch/gatemate/bitstream.cc @@ -519,9 +519,7 @@ struct BitstreamBackend void GateMateImpl::write_bitstream(const std::string &device, const std::string &filename) { - std::ofstream out(filename); - if (!out) - log_error("Failed to open file '%s' for writing (%s).\n", filename.c_str(), strerror(errno)); + auto out = open_ofstream_and_log_error(filename, "bitstream file"); BitstreamBackend be(ctx, this, device, out); be.write_bitstream(); diff --git a/himbaechel/uarch/gatemate/ccf.cc b/himbaechel/uarch/gatemate/ccf.cc index ab513161..22aebe78 100644 --- a/himbaechel/uarch/gatemate/ccf.cc +++ b/himbaechel/uarch/gatemate/ccf.cc @@ -411,9 +411,7 @@ struct GateMateCCFReader void GateMateImpl::parse_ccf(const std::string &filename) { - std::ifstream in(filename); - if (!in) - log_error("Failed to open CCF file '%s'.\n", filename.c_str()); + auto in = open_ifstream_and_log_error(filename, "CCF file"); GateMateCCFReader reader(ctx, this, in); reader.run(); } diff --git a/himbaechel/uarch/gowin/gowin.cc b/himbaechel/uarch/gowin/gowin.cc index d1cbc58d..3a48935a 100644 --- a/himbaechel/uarch/gowin/gowin.cc +++ b/himbaechel/uarch/gowin/gowin.cc @@ -267,10 +267,8 @@ void GowinImpl::pack() { if (ctx->settings.count(ctx->id("cst.filename"))) { std::string filename = ctx->settings[ctx->id("cst.filename")].as_string(); - std::ifstream in(filename); - if (!in) { - log_error("failed to open CST file '%s'\n", filename.c_str()); - } + auto in = open_ifstream_and_log_error(filename, "CST file"); + if (!gowin_apply_constraints(ctx, in)) { log_error("failed to parse CST file '%s'\n", filename.c_str()); } diff --git a/himbaechel/uarch/ng-ultra/bitstream.cc b/himbaechel/uarch/ng-ultra/bitstream.cc index 145218a3..e4315c4d 100644 --- a/himbaechel/uarch/ng-ultra/bitstream.cc +++ b/himbaechel/uarch/ng-ultra/bitstream.cc @@ -694,9 +694,7 @@ struct BitstreamJsonBackend void NgUltraImpl::write_bitstream_json(const std::string &filename) { - std::ofstream out(filename); - if (!out) - log_error("failed to open file %s for writing (%s)\n", filename.c_str(), strerror(errno)); + auto out = open_ofstream_and_log_error(filename, "bitstream json file"); BitstreamJsonBackend be(ctx, this, out); be.write_json(); diff --git a/himbaechel/uarch/ng-ultra/csv.cc b/himbaechel/uarch/ng-ultra/csv.cc index 32e0861a..2b08cd41 100644 --- a/himbaechel/uarch/ng-ultra/csv.cc +++ b/himbaechel/uarch/ng-ultra/csv.cc @@ -37,9 +37,7 @@ NEXTPNR_NAMESPACE_BEGIN void NgUltraImpl::parse_csv(const std::string &filename) { - std::ifstream in(filename); - if (!in) - log_error("failed to open CSV file '%s'\n", filename.c_str()); + auto in = open_ifstream_and_log_error(filename, "CSV file"); log_info("Parsing CSV file..\n"); std::string line; std::string linebuf; diff --git a/himbaechel/uarch/xilinx/fasm.cc b/himbaechel/uarch/xilinx/fasm.cc index ca85b2f3..15ea4c04 100644 --- a/himbaechel/uarch/xilinx/fasm.cc +++ b/himbaechel/uarch/xilinx/fasm.cc @@ -264,8 +264,7 @@ struct FasmBackend boost::erase_all(loc, "_T1"); boost::replace_all(loc, "IOI_OLOGIC", "OLOGIC_Y"); // the replacements transformed it into : LIOI3_X0Y73.OLOGIC_Y1 - out << loc << "." - << "ZINV_T1" << std::endl; + out << loc << "." << "ZINV_T1" << std::endl; } } return; @@ -1822,9 +1821,7 @@ struct FasmBackend void XilinxImpl::write_fasm(const std::string &filename) { - std::ofstream out(filename); - if (!out) - log_error("failed to open file %s for writing (%s)\n", filename.c_str(), strerror(errno)); + auto out = open_ofstream_and_log_error(filename, "FASM file"); FasmBackend be(this->ctx, this, out); be.write_fasm(); diff --git a/himbaechel/uarch/xilinx/xdc.cc b/himbaechel/uarch/xilinx/xdc.cc index 7acbba28..8feec35f 100644 --- a/himbaechel/uarch/xilinx/xdc.cc +++ b/himbaechel/uarch/xilinx/xdc.cc @@ -36,9 +36,7 @@ NEXTPNR_NAMESPACE_BEGIN void XilinxImpl::parse_xdc(const std::string &filename) { - std::ifstream in(filename); - if (!in) - log_error("failed to open XDC file '%s'\n", filename.c_str()); + auto in = open_ifstream_and_log_error(filename, "XDC file"); log_info("Parsing XDC file...\n"); std::string line; std::string linebuf; diff --git a/ice40/arch_pybindings.cc b/ice40/arch_pybindings.cc index a65ad082..0a9615d1 100644 --- a/ice40/arch_pybindings.cc +++ b/ice40/arch_pybindings.cc @@ -32,9 +32,8 @@ NEXTPNR_NAMESPACE_BEGIN static void write_bitstream(const Context *ctx, std::string asc_file) { - std::ofstream out(asc_file); - if (!out) - log_error("Failed to open output file %s\n", asc_file.c_str()); + auto out = open_ofstream_and_log_error(asc_file, "output file"); + write_asc(ctx, out); } diff --git a/ice40/main.cc b/ice40/main.cc index 20b4dee7..0d645604 100644 --- a/ice40/main.cc +++ b/ice40/main.cc @@ -98,7 +98,7 @@ void Ice40CommandHandler::customAfterLoad(Context *ctx) { if (vm.count("pcf")) { std::string filename = vm["pcf"].as(); - std::ifstream pcf(filename); + auto pcf = open_ifstream_and_log_error(filename, "PCF file"); if (!apply_pcf(ctx, filename, pcf)) log_error("Loading PCF failed.\n"); } else { @@ -109,7 +109,7 @@ void Ice40CommandHandler::customBitstream(Context *ctx) { if (vm.count("asc")) { std::string filename = vm["asc"].as(); - std::ofstream f(filename); + auto f = open_ofstream_and_log_error(filename, "ASC file"); write_asc(ctx, f); } } @@ -121,7 +121,7 @@ void Ice40CommandHandler::setupArchContext(Context *ctx) if (vm.count("read")) { std::string filename = vm["read"].as(); - std::ifstream f(filename); + auto f = open_ifstream_and_log_error(filename, "ASC file"); if (!read_asc(ctx, f)) log_error("Loading ASC failed.\n"); } diff --git a/machxo2/bitstream.cc b/machxo2/bitstream.cc index 6966c455..ec44bc30 100644 --- a/machxo2/bitstream.cc +++ b/machxo2/bitstream.cc @@ -797,7 +797,7 @@ void write_bitstream(Context *ctx, std::string text_config_file) // Configure chip if (!text_config_file.empty()) { - std::ofstream out_config(text_config_file); + auto out_config = open_ofstream_and_log_error(text_config_file, "text config file"); out_config << bitgen.cc; } } diff --git a/machxo2/main.cc b/machxo2/main.cc index ae9af7a7..f532c4d6 100644 --- a/machxo2/main.cc +++ b/machxo2/main.cc @@ -89,9 +89,7 @@ void MachXO2CommandHandler::customAfterLoad(Context *ctx) if (vm.count("lpf")) { std::vector files = vm["lpf"].as>(); for (const auto &filename : files) { - std::ifstream in(filename); - if (!in) - log_error("failed to open LPF file '%s'\n", filename.c_str()); + auto in = open_ifstream_and_log_error(filename, "LPF file"); if (!ctx->apply_lpf(filename, in)) log_error("failed to parse LPF file '%s'\n", filename.c_str()); } diff --git a/mistral/main.cc b/mistral/main.cc index 232655b0..d6d9333c 100644 --- a/mistral/main.cc +++ b/mistral/main.cc @@ -17,6 +17,7 @@ * */ +#include #include #include "command.h" #include "design_utils.h" @@ -62,8 +63,10 @@ void MistralCommandHandler::customBitstream(Context *ctx) ctx->cyclonev->rbf_save(data); std::ofstream out(filename, std::ios::binary); - if (!out) - log_error("Failed to open output RBF file %s.\n", filename.c_str()); + if (!out.is_open()) { + log_error("Failed to open RBF file '%s' for writing: %s.\n", filename.c_str(), + std::error_code(errno, std::generic_category()).message().c_str()); + } out.write(reinterpret_cast(data.data()), data.size()); } } @@ -85,9 +88,7 @@ void MistralCommandHandler::customAfterLoad(Context *ctx) { if (vm.count("qsf")) { std::string filename = vm["qsf"].as(); - std::ifstream in(filename); - if (!in) - log_error("Failed to open input QSF file %s.\n", filename.c_str()); + auto in = open_ifstream_and_log_error(filename, "input QSF file"); ctx->read_qsf(in); } } diff --git a/nexus/main.cc b/nexus/main.cc index 66282c90..c3302d88 100644 --- a/nexus/main.cc +++ b/nexus/main.cc @@ -62,9 +62,7 @@ void NexusCommandHandler::customBitstream(Context *ctx) { if (vm.count("fasm")) { std::string filename = vm["fasm"].as(); - std::ofstream out(filename); - if (!out) - log_error("Failed to open output FASM file %s.\n", filename.c_str()); + auto out = open_ofstream_and_log_error(filename, "output FASM file"); ctx->write_fasm(out); } } @@ -101,9 +99,7 @@ void NexusCommandHandler::customAfterLoad(Context *ctx) { if (vm.count("pdc")) { std::string filename = vm["pdc"].as(); - std::ifstream in(filename); - if (!in) - log_error("Failed to open input PDC file %s.\n", filename.c_str()); + auto in = open_ifstream_and_log_error(filename, "input PDC file"); ctx->read_pdc(in); } }