From 6a7b7272cd6ce6b8264796c25b686bf5376b8aea Mon Sep 17 00:00:00 2001 From: Edward Welbourne Date: Mon, 4 Sep 2023 17:55:47 +0200 Subject: [PATCH] Deduplicate code in setup of serialization converters MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The setup of the input and output converters is fairly complex but was made harder to read by nearly-duplicating its logic for input and for output. Break out into a separate function to make clear what parts differ between the two and what parts don't. In the process, allow the search for a named converter to continue past a match that doesn't support the needed direction, on the off chance of a name collision. Make an error message more articulate in the process. Pick-to: 6.6 6.5 Task-number: QTBUG-111228 Change-Id: I2a9de8b406c538098076f388fc8a1980b91fe16b Reviewed-by: MÃ¥rten Nordheim --- .../corelib/serialization/convert/main.cpp | 131 +++++++----------- 1 file changed, 49 insertions(+), 82 deletions(-) diff --git a/examples/corelib/serialization/convert/main.cpp b/examples/corelib/serialization/convert/main.cpp index ddb5a49eed..ede03dbe1d 100644 --- a/examples/corelib/serialization/convert/main.cpp +++ b/examples/corelib/serialization/convert/main.cpp @@ -1,4 +1,5 @@ // Copyright (C) 2018 Intel Corporation. +// Copyright (C) 2023 The Qt Company Ltd. // SPDX-License-Identifier: LicenseRef-Qt-Commercial OR BSD-3-Clause #include "converter.h" @@ -27,6 +28,50 @@ Converter::~Converter() availableConverters->removeAll(this); } +static const Converter *prepareConverter(QString format, Converter::Direction direction, + QFile *stream) +{ + const bool out = direction == Converter::Direction::Out; + const QIODevice::OpenMode mode = out + ? QIODevice::WriteOnly | QIODevice::Truncate + : QIODevice::ReadOnly; + const char *dirn = out ? "output" : "input"; + + if (stream->fileName().isEmpty()) + stream->open(out ? stdout : stdin, mode); + else + stream->open(mode); + + if (!stream->isOpen()) { + fprintf(stderr, "Could not open \"%s\" for %s: %s\n", + qPrintable(stream->fileName()), dirn, qPrintable(stream->errorString())); + } else if (format == "auto"_L1) { + for (const Converter *conv : std::as_const(*availableConverters)) { + if (conv->directions().testFlag(direction) && conv->probeFile(stream)) + return conv; + } + if (out) // Failure to identify output format can be remedied by loadFile(). + return nullptr; + + // Input format, however, we must know before we can call that: + fprintf(stderr, "Could not determine input format. Specify it with the -I option.\n"); + } else { + for (const Converter *conv : std::as_const(*availableConverters)) { + if (conv->name() == format) { + if (!conv->directions().testFlag(direction)) { + fprintf(stderr, "File format \"%s\" cannot be used for %s\n", + qPrintable(format), dirn); + continue; // on the off chance there's another with the same name + } + return conv; + } + } + fprintf(stderr, "Unknown %s file format \"%s\"\n", dirn, qPrintable(format)); + } + exit(EXIT_FAILURE); + Q_UNREACHABLE_RETURN(nullptr); +} + int main(int argc, char *argv[]) { QCoreApplication app(argc, argv); @@ -103,91 +148,13 @@ int main(int argc, char *argv[]) return EXIT_FAILURE; } - const Converter *inconv = nullptr; - QString format = parser.value(inputFormatOption); - if (format != "auto"_L1) { - for (const Converter *conv : std::as_const(*availableConverters)) { - if (conv->name() == format) { - inconv = conv; - break; - } - } - - if (!inconv || !inconv->directions().testFlag(Converter::Direction::In)) { - fprintf(stderr, inconv ? "File format \"%s\" cannot be used for input\n" - : "Unknown input file format \"%s\"\n", qPrintable(format)); - return EXIT_FAILURE; - } - } - - const Converter *outconv = nullptr; - format = parser.value(outputFormatOption); - if (format != "auto"_L1) { - for (const Converter *conv : std::as_const(*availableConverters)) { - if (conv->name() == format) { - outconv = conv; - break; - } - } - - if (!outconv || !outconv->directions().testFlag(Converter::Direction::Out)) { - fprintf(stderr, outconv ? "File format \"%s\" cannot be used for output\n" - : "Unknown output file format \"%s\"\n", qPrintable(format)); - return EXIT_FAILURE; - } - } - QStringList files = parser.positionalArguments(); QFile input(files.value(0)); QFile output(files.value(1)); - - if (input.fileName().isEmpty()) - input.open(stdin, QIODevice::ReadOnly); - else - input.open(QIODevice::ReadOnly); - if (!input.isOpen()) { - fprintf(stderr, "Could not open \"%s\" for reading: %s\n", - qPrintable(input.fileName()), qPrintable(input.errorString())); - return EXIT_FAILURE; - } - - if (output.fileName().isEmpty()) - output.open(stdout, QIODevice::WriteOnly | QIODevice::Truncate); - else - output.open(QIODevice::WriteOnly | QIODevice::Truncate); - if (!output.isOpen()) { - fprintf(stderr, "Could not open \"%s\" for writing: %s\n", - qPrintable(output.fileName()), qPrintable(output.errorString())); - return EXIT_FAILURE; - } - - if (!inconv) { - // probe the input to find a file format - for (const Converter *conv : std::as_const(*availableConverters)) { - if (conv->directions().testFlag(Converter::Direction::In) - && conv->probeFile(&input)) { - inconv = conv; - break; - } - } - - if (!inconv) { - fprintf(stderr, "Could not determine input format. pass -I option.\n"); - return EXIT_FAILURE; - } - } - - if (!outconv) { - // probe the output to find a file format - for (const Converter *conv : std::as_const(*availableConverters)) { - if (conv->directions().testFlag(Converter::Direction::Out) - && conv->probeFile(&output)) { - outconv = conv; - break; - } - } - // If that failed, loadFile() shall supply a fallback. - } + const Converter *inconv = prepareConverter(parser.value(inputFormatOption), + Converter::Direction::In, &input); + const Converter *outconv = prepareConverter(parser.value(outputFormatOption), + Converter::Direction::Out, &output); // now finally perform the conversion QVariant data = inconv->loadFile(&input, outconv);