inih

inih copied to clipboard

MSVC 14.32.3132 throws C4244 when compiling INIReader.cpp.

1>C:\Program Files\Microsoft Visual Studio\2022\Community\VC\Tools\MSVC\14.32.31326\include\algorithm(3031,24): warning C4244: '=': conversion from 'int' to 'char', possible loss of data
1>...\Lib\inih\cpp\INIReader.cpp(72): message : see reference to function template instantiation '_OutIt std::transform<std::_String_iterator<std::_String_val<std::_Simple_types<_Elem>>>,std::_String_iterator<std::_String_val<std::_Simple_types<_Elem>>>,int(__cdecl *)(int)>(const _InIt,const _InIt,_OutIt,_Fn)' being compiled
1>        with
1>        [
1>            _OutIt=std::_String_iterator<std::_String_val<std::_Simple_types<char>>>,
1>            _Elem=char,
1>            _InIt=std::_String_iterator<std::_String_val<std::_Simple_types<char>>>,
1>            _Fn=int (__cdecl *)(int)
1>        ]

This PR fixes this, by using a lambda which static casts the return value of ::tolower into an unsigned char.

TLDR:

• ::tolower returns int
• MSVC throws the warning C4244: conversion from 'int' to 'char'
• this lambda static casts the return value back to char

Thanks for the contribution. I've been out of C++ for so long I don't really know how to evaluate this solution. Given that inih is stated as "good for embedded systems", is the use of C++11 okay here? Maybe some others who are more well-versed in C++ can comment on the appropriateness of this solution for inih.

Hi @benhoyt You can have a look here: https://en.cppreference.com/w/cpp/algorithm/transform std::transform expects a lambda or function as last parameter. Since lambdas were introduced in C++11 this should be compatible. Also the std::transform example shows a lambda as last parameter. But of course I'd be glad to hear some feedback from other C++ devs :)

Thanks. I'm going to ping the recent (in the last few years!) contributors to the C++ wrapper for inih: @vrresto, @evorw, @NeatNit, @zhaowenlan1779, @Jvanrhijn, @FredrikBlix -- I'm assuming you're a lot more familiar with C++ than I am. Could a couple of you please comment on this change?

It looks fine to me, the code also matches up with example usage of both std::transform and std::tolower on cppreference - unless there's some super subtle issue I'm missing (it's also been a couple of years since I've last used C++).

Thanks @AbsintheScripting, and sorry for the delay merging.