Hard to use this API "Highs_getStringOptionValue"

[mingodad]

While trying to use this API Highs_getStringOptionValue I noticed that it's not easy or well designed because the user do not know how big the allocation need to be beforehand, probably would be better to return an allocated string instead and the user will be responsible to free it.

Or maybe add another parameter to pass the allocated size and also to return the string size, so we can check if we have allocated enough or not by checking the in_out_size if we get an error:
Something like:

HighsInt Highs_getStringOptionValue(const void* highs, const char* option,
                                    char* value, HighsInt *in_out_size);

Actual implementation:

HighsInt Highs_getStringOptionValue(const void* highs, const char* option,
                                    char* value) {
  std::string v;
  memset(value, 0, 7);
  HighsInt retcode =
      (HighsInt)((Highs*)highs)->getOptionValue(std::string(option), v);
  strcpy(value, v.c_str());
  return retcode;
}

[mingodad]

Or even better:

HighsInt Highs_getStringOptionValue(const void* highs, const char* option,
                                    char* const* out_value, HighsInt *out_size);

Returning a pointer to a const string and it's size and then the user should create a copy to store it.

[mingodad]

There is also this related issue #764

[odow]

Returning a pointer to a const string and it's size and then the user should create a copy to store it.

This is probably the best option. Similar to what is used in Gurobi:
https://www.gurobi.com/documentation/10.0/refman/c_getstrattr.html

In HiGHS.jl I just used an arbitrarily large buffer (1024):
https://github.com/jump-dev/HiGHS.jl/blob/11aec480e18de9532bfe1454c84390b5ffeef65c/src/MOI_wrapper.jl#L541-L544

I guess we could improve things by having a constant like GRB_MAX_STRLEN:
https://github.com/jump-dev/Gurobi.jl/blob/44101cc909bdd6d86a91da1770d754c28ec6030f/src/gen100/libgrb_common.jl#L67,
and then throw if provided options have a greater length than that.

We also need to be careful not to make a breaking change to the C API.

[mingodad]

I don't think that we need to be careful not to make a breaking change to the C API when it's clearly have issues that will make it difficult to use.

[jajhall]

Thanks for the observation. This has been a feature of the C API for some time, and no-one else has commented on it, so fixing it isn't a priority. Plus, as @odow observes, we'd be making a breaking change to the C API. We'll modify this when we create HiGHS v2.0

[odow]

when it's clearly have issues that will make it difficult to use.

How often does the user call getStringOptionValue though? Allocating a large buffer seems okay.

For most of the options, the strings are short, like off, on, choose:

HiGHS/src/lp_data/HighsOptions.h

Lines 447 to 468 in 0329e0c

	record_string = new OptionRecordString(
	kPresolveString, "Presolve option: \"off\", \"choose\" or \"on\"",
	advanced, &presolve, kHighsChooseString);
	records.push_back(record_string);
	record_string = new OptionRecordString(
	kSolverString,
	"Solver option: \"simplex\", \"choose\" or \"ipm\". If "
	"\"simplex\"/\"ipm\" is chosen then, for a MIP (QP) the integrality "
	"constraint (quadratic term) will be ignored",
	advanced, &solver, kHighsChooseString);
	records.push_back(record_string);
	record_string = new OptionRecordString(
	kParallelString, "Parallel option: \"off\", \"choose\" or \"on\"",
	advanced, &parallel, kHighsChooseString);
	records.push_back(record_string);
	record_string = new OptionRecordString(
	kRunCrossoverString, "Run IPM crossover: \"off\", \"choose\" or \"on\"",
	advanced, &run_crossover, kHighsOnString);
	records.push_back(record_string);

The arbitrary length strings are the solution file and log file:

HiGHS/src/lp_data/HighsOptions.h

Lines 635 to 642 in 0329e0c

	record_string =
	new OptionRecordString(kSolutionFileString, "Solution file", advanced,
	&solution_file, kHighsFilenameDefault);
	records.push_back(record_string);
	record_string = new OptionRecordString(kLogFileString, "Log file", advanced,
	&log_file, "");
	records.push_back(record_string);

But you control the user calling these, so throw an error if they pass a string longer than 1024 characters (for example).