Re: Fwd: [ABANDONED] Remove some excessive log formatting

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 :)