emilk / loguru

A lightweight C++ logging library
The Unlicense
1.76k stars 256 forks source link

Change `buff_size` param, `unsigned` to `size_t` #212

Closed skaravos closed 2 years ago

skaravos commented 2 years ago

Notes:

bylowerik commented 2 years ago

The error you refer to says it is a problem to have unsigned != size_t. Is it ok if type > size_t? Or is always size_t == unsigned long long?

bylowerik commented 2 years ago

Otherwise lgtm

skaravos commented 2 years ago

Is it ok if type > size_t? Or is always size_t == unsigned long long?

size_t == unsigned long long on almost all 64bit systems. size_t < unsigned long long on 32bit systems.

However, even on a 32bit system this shouldn't cause any major troubles. On such a system, the compiler may issue a warning about potential loss of data because it will implicitly downcast the buff_size variable into a size_t in the snprintf() call.

IMHO, its better to overshoot the value of size_t, rather than undershoot because undershooting causes compiler warnings on more systems.

Overall this solution is a bit of a compromise. Ideally, the declaration and definition would both use size_t and the underlying type would be determined by the platform. However, this would require the header file to #include <cstddef>, which goes against the philosophy of the project.