Fwd: Re: [PATCH] Fix uninitialized variable with wrong type
Yuriy Skalko
yuriy.skalko at gmail.com
Fri Sep 11 10:30:09 UTC 2020
> I like the patch. Did you manage to test it ? I do not know how to check that errors are caught.
Thanks. I tested it in my normal LyX usage on Windows. How to provide
more thorough testing?
> The following code
> + bool valid = (pret == 0);
> + if (!success)
> + valid = false;
> should be replaced with
> + bool valid = (pret == 0) && valid;
>
> I would also prefer to have the definition of the struct in multi-line format.
You've meant "&& success", right? I've done the corrections.
> Concerning the more ambitious FIXME, I have to admit that I have not researched this at all, and that we can just forget about it for now.
>
>
> JMarc
-------------- next part --------------
From 87e2d68f5abe40315efab10b35c4d7a8b0bc095a Mon Sep 17 00:00:00 2001
From: Yuriy Skalko <yuriy.skalko at gmail.com>
Date: Fri, 11 Sep 2020 00:22:55 +0300
Subject: [PATCH] Refactor runCommand
---
src/Buffer.cpp | 2 +-
src/TextClass.cpp | 2 +-
src/mathed/MathExtern.cpp | 2 +-
src/support/filetools.cpp | 28 +++++++++++++++-------------
src/support/filetools.h | 5 ++++-
src/support/os.cpp | 4 ++--
src/support/os_win32.cpp | 4 ++--
7 files changed, 26 insertions(+), 21 deletions(-)
diff --git a/src/Buffer.cpp b/src/Buffer.cpp
index 9de281daf3..82fd0899c4 100644
--- a/src/Buffer.cpp
+++ b/src/Buffer.cpp
@@ -1357,7 +1357,7 @@ Buffer::ReadStatus Buffer::convertLyXFormat(FileName const & fn,
LYXERR(Debug::INFO, "Running '" << command_str << '\'');
cmd_ret const ret = runCommand(command_str);
- if (ret.first != 0) {
+ if (!ret.valid) {
if (from_format < LYX_FORMAT) {
Alert::error(_("Conversion script failed"),
bformat(_("%1$s is from an older version"
diff --git a/src/TextClass.cpp b/src/TextClass.cpp
index eb92612d84..4cc5eba0ed 100644
--- a/src/TextClass.cpp
+++ b/src/TextClass.cpp
@@ -92,7 +92,7 @@ bool layout2layout(FileName const & filename, FileName const & tempfile,
LYXERR(Debug::TCLASS, "Running `" << command_str << '\'');
cmd_ret const ret = runCommand(command_str);
- if (ret.first != 0) {
+ if (!ret.valid) {
if (format == LAYOUT_FORMAT)
LYXERR0("Conversion of layout with layout2layout.py has failed.");
return false;
diff --git a/src/mathed/MathExtern.cpp b/src/mathed/MathExtern.cpp
index 311b323895..18340e11c8 100644
--- a/src/mathed/MathExtern.cpp
+++ b/src/mathed/MathExtern.cpp
@@ -1026,7 +1026,7 @@ namespace {
<< "\ninput: '" << data << "'" << endl;
cmd_ret const ret = runCommand(command);
cas_tmpfile.removeFile();
- return ret.second;
+ return ret.result;
}
size_t get_matching_brace(string const & str, size_t i)
diff --git a/src/support/filetools.cpp b/src/support/filetools.cpp
index d6b27856cb..8ffca684a9 100644
--- a/src/support/filetools.cpp
+++ b/src/support/filetools.cpp
@@ -1095,38 +1095,40 @@ cmd_ret const runCommand(string const & cmd)
// (Claus Hentschel) Check if popen was successful ;-)
if (!inf) {
lyxerr << "RunCommand:: could not start child process" << endl;
- return make_pair(-1, string());
+ return {false, string()};
}
- string ret;
+ string result;
int c = fgetc(inf);
while (c != EOF) {
- ret += static_cast<char>(c);
+ result += static_cast<char>(c);
c = fgetc(inf);
}
#if defined (_WIN32)
WaitForSingleObject(process.hProcess, INFINITE);
DWORD pret;
- if (!GetExitCodeProcess(process.hProcess, &pret))
- pret = -1;
+ BOOL success = GetExitCodeProcess(process.hProcess, &pret);
+ bool valid = (pret == 0) && success;
if (!infile.empty())
CloseHandle(startup.hStdInput);
CloseHandle(process.hProcess);
if (fclose(inf) != 0)
- pret = -1;
+ valid = false;
#elif defined (HAVE_PCLOSE)
int const pret = pclose(inf);
+ bool valid = (pret != -1);
#elif defined (HAVE__PCLOSE)
int const pret = _pclose(inf);
+ bool valid = (pret != -1);
#else
#error No pclose() function.
#endif
- if (pret == -1)
+ if (!valid)
perror("RunCommand:: could not terminate child process");
- return make_pair(pret, ret);
+ return {valid, result};
}
@@ -1174,10 +1176,10 @@ FileName const findtexfile(string const & fil, string const & /*format*/,
cmd_ret const c = runCommand(kpsecmd);
- LYXERR(Debug::LATEX, "kpse status = " << c.first << '\n'
- << "kpse result = `" << rtrim(c.second, "\n\r") << '\'');
- if (c.first != -1)
- return FileName(rtrim(to_utf8(from_filesystem8bit(c.second)), "\n\r"));
+ LYXERR(Debug::LATEX, "kpse status = " << c.valid << '\n'
+ << "kpse result = `" << rtrim(c.result, "\n\r") << '\'');
+ if (c.valid)
+ return FileName(rtrim(to_utf8(from_filesystem8bit(c.result)), "\n\r"));
else
return FileName();
}
@@ -1223,7 +1225,7 @@ bool prefs2prefs(FileName const & filename, FileName const & tempfile, bool lfun
LYXERR(Debug::FILES, "Running `" << command_str << '\'');
cmd_ret const ret = runCommand(command_str);
- if (ret.first != 0) {
+ if (!ret.valid) {
LYXERR0("Could not run file conversion script prefs2prefs.py.");
return false;
}
diff --git a/src/support/filetools.h b/src/support/filetools.h
index b0f5f7f379..e66af9972f 100644
--- a/src/support/filetools.h
+++ b/src/support/filetools.h
@@ -328,7 +328,10 @@ bool prefs2prefs(FileName const & filename, FileName const & tempfile,
/// Does file \p file need to be updated by configure.py?
bool configFileNeedsUpdate(std::string const & file);
-typedef std::pair<int, std::string> cmd_ret;
+struct cmd_ret {
+ bool valid;
+ std::string result;
+};
cmd_ret const runCommand(std::string const & cmd);
diff --git a/src/support/os.cpp b/src/support/os.cpp
index 616b9c6936..3a5c2e59a2 100644
--- a/src/support/os.cpp
+++ b/src/support/os.cpp
@@ -65,7 +65,7 @@ static string const python23_call(string const & binary, bool verbose = false)
smatch sm;
try {
static regex const python_reg("\\((\\d*), (\\d*)\\)");
- if (out.first < 0 || !regex_match(out.second, sm, python_reg))
+ if (!out.valid || !regex_match(out.result, sm, python_reg))
return string();
} catch(regex_error const & /*e*/) {
LYXERR0("Regex error! This should not happen.");
@@ -78,7 +78,7 @@ static string const python23_call(string const & binary, bool verbose = false)
return string();
if (verbose)
- lyxerr << "Found Python " << out.second << "\n";
+ lyxerr << "Found Python " << out.result << "\n";
// Add the -tt switch so that mixed tab/whitespace
// indentation is an error
return binary + " -tt";
diff --git a/src/support/os_win32.cpp b/src/support/os_win32.cpp
index e4f9ebd0fc..536d2a6446 100644
--- a/src/support/os_win32.cpp
+++ b/src/support/os_win32.cpp
@@ -229,8 +229,8 @@ void init(int argc, char ** argv[])
// to the outer split which sets cygdrive with its
// contents until the first blank, discarding the
// unneeded return value.
- if (p.first != -1 && prefixIs(p.second, "Prefix"))
- split(split(p.second, '\n'), cygdrive, ' ');
+ if (p.valid && prefixIs(p.result, "Prefix"))
+ split(split(p.result, '\n'), cygdrive, ' ');
}
}
--
2.28.0.windows.1
More information about the lyx-devel
mailing list