Fwd: [ABANDONED] Remove some excessive log formatting
Stephan Bergmann
sbergman at redhat.com
Fri Nov 22 13:02:16 UTC 2019
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 :)
More information about the LibreOffice
mailing list