Skip to content

Commit f65b89e

Browse files
committed
Use Highs::closeLogFile in destructor
1 parent bad8b57 commit f65b89e

File tree

3 files changed

+20
-23
lines changed

3 files changed

+20
-23
lines changed

highs/Highs.h

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -46,11 +46,9 @@ class Highs {
4646
FILE* log_stream = this->options_.log_options.log_stream;
4747
if (log_stream != nullptr) {
4848
assert(log_stream != stdout);
49-
// Was this, but only passing a copy of the pointer, so not
50-
// closing the true log stream?
51-
//
52-
// fclose(log_stream);
53-
fclose(this->options_.log_options.log_stream);
49+
// Flush and close log_stream
50+
fflush(log_stream);
51+
fclose(log_stream);
5452
}
5553
}
5654

highs/lp_data/Highs.cpp

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4763,20 +4763,16 @@ HighsStatus Highs::openLogFile(const std::string& log_file) {
47634763
}
47644764

47654765
HighsStatus Highs::closeLogFile() {
4766+
// Work with a copy of the pointer for brevity
47664767
FILE* log_stream = this->options_.log_options.log_stream;
47674768
if (log_stream != nullptr) {
47684769
assert(log_stream != stdout);
4769-
// Using this, as in the original destructor of Highs, only
4770-
// passing a copy of the pointer, so not closing the true log
4771-
// stream?
4772-
//
4773-
// fclose(log_stream);
4774-
fclose(this->options_.log_options.log_stream);
4775-
// Set log_stream to nullptr to give a test whether the it has
4776-
// been closed (and avoid trying to close it again which causes an
4777-
// error).
4770+
fflush(log_stream);
4771+
fclose(log_stream);
4772+
// Set the true log_stream to nullptr to give a test whether it
4773+
// has been closed (and avoid trying to close it again which
4774+
// causes an error)
47784775
this->options_.log_options.log_stream = nullptr;
4779-
assert(this->options_.log_options.log_stream == nullptr);
47804776
}
47814777
return HighsStatus::kOk;
47824778
}

highs/lp_data/HighsOptions.cpp

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -25,19 +25,22 @@ void highsOpenLogFile(HighsLogOptions& log_options,
2525
OptionStatus status =
2626
getOptionIndex(log_options, "log_file", option_records, index);
2727
assert(status == OptionStatus::kOk);
28-
if (log_options.log_stream != NULL) {
28+
if (log_options.log_stream != nullptr) {
2929
// Current log file stream is not null, so flush and close it
3030
fflush(log_options.log_stream);
3131
fclose(log_options.log_stream);
32+
// Set the stream to null to give a test whether it has been
33+
// closed (and avoid trying to close it again which causes an
34+
// error)
35+
log_options.log_stream = nullptr;
3236
}
33-
if (log_file.compare("")) {
34-
// New log file name is not empty, so open it, appending if
35-
// possible
37+
assert(!log_options.log_stream);
38+
// If new log file name is not empty, open it, appending if possible
39+
//
40+
// If fopen fails then it returns nullptr, so log_file is open for
41+
// writing or nullptr
42+
if (log_file.compare(""))
3643
log_options.log_stream = fopen(log_file.c_str(), "a");
37-
} else {
38-
// New log file name is empty, so set the stream to null
39-
log_options.log_stream = NULL;
40-
}
4144
OptionRecordString& option = *(OptionRecordString*)option_records[index];
4245
option.assignvalue(log_file);
4346
}

0 commit comments

Comments
 (0)