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

possible API bug short versa unsigned short #219

Closed aotto1968 closed 1 year ago

aotto1968 commented 2 years ago

Hi,

I using libconfig in my project and ONE task is to write an API for different languages … Now I figure out a problem with the API function "config_set_default_format"

in libconfig.h this function has the prototype :

extern LIBCONFIG_API void config_set_default_format(config_t *config,
                                                    short format);

and the implementation as macro :

#define  /* void */ config_set_default_format(/* config_t * */ C,       \
                                              /* short */ F)            \
  (C)->default_format = (F)

but the "short" parameter is in real "unsigned short"

typedef struct config_t
{
  config_setting_t *root;
  void (*destructor)(void *);
  int options;
  unsigned short tab_width;
  unsigned short float_precision;
  unsigned short default_format;    <<<<<<<<<<<<<<<<<<<<<<<
  const char *include_dir;
  config_include_fn_t include_fn;
  const char *error_text;
  const char *error_file;
  int error_line;
  config_error_t error_type;
  const char **filenames;
  void *hook;
} config_t;

and my "automatic" generated API fails to compile.

MqSettingDocC_tcl.c: In function 'tclmsgque_MqSettingDocC_SetDefaultFormat':
MqSettingDocC_tcl.c|363 col 3| error: conversion to 'short unsigned int' from 'MQ_SRT' {aka 'short int'} may change the sign of the result [-Werror=sign-conversion]                                                                                         
||   363 |   config_set_default_format (hdl->nat, format);
||       |   ^~~~~~~~~~~~~~~~~~~~~~~~~
|| compilation terminated due to -Wfatal-errors.

just to be truth, I don't like "unsigned" at all

thomastrapp commented 2 years ago

I think the best course of action is to change the implicit cast in the macro to an explicit cast. The legal values for format are 0 and 1.

diff --git a/lib/libconfig.h b/lib/libconfig.h
index 5e4eea3..6a2ac52 100644
--- a/lib/libconfig.h
+++ b/lib/libconfig.h
@@ -323,10 +323,10 @@ extern LIBCONFIG_API int config_lookup_string(const config_t *config,

 #define  /* void */ config_set_default_format(/* config_t * */ C,       \
                                               /* short */ F)            \
-  (C)->default_format = (F)
+  (C)->default_format = (unsigned short)(F)

 #define /* short */ config_get_default_format(/* config_t * */ C)       \
-  ((C)->default_format)
+  ((short)(C)->default_format)

 #define /* unsigned short */ config_setting_source_line(   \
   /* const config_setting_t * */ S)                        \

@aotto1968 Does this patch solve the issue for you?

hyperrealm commented 1 year ago

Fixed in 935062924d2031de52c5e171cc8ef72cce0785e2