On 22/11/2019 12:13, Luboš Luňák wrote:
On Thursday 21 of November 2019, Stephan Bergmann wrote:
* Too much trivial SAL_INFO noise (that was once added ad-hoc and should
arguably have only been in a developer's local build) can make it
unnecessarily hard to see the more relevant SAL_INFOs (and log area
granularity is limited in practice). Of course, it is hard to tell how
much of the existing SAL_INFO use falls under that noise category.
If I'm not mistaken, all SAL_INFO is disabled by default, which trivially
solves that.
I meant the scenario where you enable one specific +INFO.area and the
noise ratio for that area is high (like is the case for sal.osl, from my
personal experience).
When I had originally designed SAL_WARN/SAL_INFO, I had anticipated use
along the lines of:
* Use SAL_WARN if it is an obviously erroneous unusual event. Like
SAL_WARN_IF(
*ppLibraryUrl == nullptr, "sal.osl", "rtl_string2UString
failed");
in osl_getModuleURLFromAddress (sal/osl/unx/module.cxx), where it warns
if the passed-in ppLibraryUrl cannot be converted to an 8-bit string.
I think that if something is an obvious error, then the code should just
plain assert. IMO the codebase is way too defensive and it's one of the
reasons why there are so many warnings that nobody cares about - because they
do not have to.
Assert is for programming errors, not for cases like the above where the
8-bit pathname obtained from dladdr(3) may contain bytes incompatible
with osl_getThreadTextEncoding().
(Problems with our defensive code base notwithstanding.)
I do not claim that this is the only reasonable approach to using
SAL_WARN/SAL_INFO, and I have seen more liberal use effectively from day
one. But I'm always a little concerned, for the reasons outlined above,
when I come across what looks like overly liberal use of SAL_INFO.
One way to address your concerns might be by adding SAL_TRACE (or whatever
it'd be named), which would be even one level lower than SAL_INFO and it
wouldn't even be compiled in release builds if we get to the point of keeping
SAL_INFO for those. But when we got to looking at the actual issue, how this
would actually matter. The purpose of keeping SAL_INFO in release builds
would be presumably that users could provide such output when asked for e.g.
in bugzilla, but then how to decide where to draw the line? If I want the
debug output, I probably want all of it.
And of course all of this matters only if it actually matters in practice.
I'm still fairly convinced that both the space and time cost can be
negligible, you have designed it to be pretty cheap if not used (with the
possible exception of sal_detail_log_report(), but that could be improved if
needed).
That's why I wrote that I'm just "a little concerned"; nothing much to
worry about :)
_______________________________________________
LibreOffice mailing list
LibreOffice@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/libreoffice
