hyperrealm / libconfig

C/C++ library for processing configuration files
https://hyperrealm.github.io/libconfig/
GNU Lesser General Public License v2.1
1.1k stars 359 forks source link

Config::lookupValue(const char *path, long long &value) always false #214

Closed mastersan-mh closed 1 year ago

mastersan-mh commented 2 years ago

Part 1

config.conf:

values:
{
var = 50;
}
libconfig::Config cfg;
cfg.readFile("config.conf");
int var1;
long long var2;
bool x1 = cfg.lookupValue("values.var", var1);
bool x2 = cfg.lookupValue("values.var", var2);

x2 always false, even if x1 is true.

If we write like this:

libconfig::Config cfg;
cfg.readFile("config.conf");
cfg.setAutoConvert(true); // Added line
int var1;
long long var2;
bool x1 = cfg.lookupValue("values.var", var1);
bool x2 = cfg.lookupValue("values.var", var2);

we can get x2 == true.

At the same time, C functions config_setting_lookup_int() config_setting_lookup_int64() work properly, and we can get "values.var" by any of this functions.

Part 2

But there is still no way to get var2 value > 2^31, as long long type allows. Anyway, code on C with using function config_setting_lookup_int64() works properly, and we can get value > 2^31.

I think, libconfig++ has broken behavior in compare with libconfig.

thomastrapp commented 2 years ago

I can reproduce your issue.

The documentation on 64-bit integers:

Long long (64-bit) integers are represented identically to integers, except that an ‘L’ character is appended to indicate a 64-bit value. For example, ‘0L’ indicates a 64-bit integer value 0. As of version 1.5 of the library, the trailing ‘L’ is optional; if the integer value exceeds the range of a 32-bit integer, it will automatically be interpreted as a 64-bit integer.

The integer and 64-bit integer setting types are interchangeable to the extent that a conversion between the corresponding native types would not result in an overflow or underflow. For example, a long long value can be written to a setting that has an integer type, if that value is within the range of an int. This rule applies to every API function or method that reads a value from or writes a value to a setting: if the type conversion would not result in an overflow or underflow, then the call will succeed, and otherwise it will fail. This behavior was not well-defined prior to version 1.7 of the library.

My interpretation of the documentation is that your example should work: The smaller integer type (var=50;) should be implicitly converted to an integer of type long long (when calling lookupValue(..., long long&)), even with OptionAutoConvert disabled.

#include <cstdlib>
#include <iostream>
#include <libconfig.h++>

int main()
{
  libconfig::Config cfg;
  cfg.readString("var=50;"); // no trailing 'L'

  long long var = 0;
  // This should work, but it doesn't:
  bool ret = cfg.lookupValue("var", var);

  std::cout << std::boolalpha
            << "ret: " << ret << "\n"  // false
            << "var: " << var << "\n"; // 0

  return EXIT_SUCCESS;
}

The following patch fixes the issue for me:

diff --git a/lib/libconfigcpp.c++ b/lib/libconfigcpp.c++
index add44f5..9b20e01 100644
--- a/lib/libconfigcpp.c++
+++ b/lib/libconfigcpp.c++
@@ -1188,7 +1188,7 @@ Setting & Setting::add(Setting::Type type)

 void Setting::assertType(Setting::Type type) const
 {
-  if(type != _type)
+  if(type != _type && !(type == TypeInt64 && _type == TypeInt))
   {
     if(!(isNumber() && config_get_auto_convert(_setting->config)
          && ((type == TypeInt) || (type == TypeInt64) || (type == TypeFloat))))

I need to check if this fix is sufficient; then I'll make a pull request.

hyperrealm commented 1 year ago

Fixed in 0a5cfd99532531500daa1b459257636d3220afc8