Re: [Libreoffice] Assertions and Logging

On 11/21/2011 04:42 PM, Caolán McNamara wrote:
> On Mon, 2011-11-21 at 14:30 +0100, Stephan Bergmann wrote:
> > On 11/21/2011 01:30 PM, Caolán McNamara wrote:
> > > Practical question though, is on windows where does the output go ?
> >
> > SAL_INFO/WARN just go to stderr for now.  What should work to see them
> > even for a gui soffice.exe is to add something like 2>log.txt to the
> > command line.
>
> oh, does that work ? I was labouring under the misunderstanding that we
> closed those streams under windows, or something of that nature. I'm a
> complete windows weenie.

I remember it at least used to work even with 4NT back in those days 
(the supported syntax there was the csh >&, IIRC).  No idea whether we 
started to close stderr for some reason since then, though.

> > > assert(pFoo);
> > > if (!pFoo)
> > >       throw catchAbleFoo("wtf");
> > >
> > > i.e. do we have a philosophical problem with gracefully/semi-gracefully
> > > handing should-be impossible cases ?
> >
> > I think that's a perversion, and should be avoided.  It the author could
> > not convince himself that !pFoo is not impossible (modulo bugs), then he
> > should use OSL_WARN instead.  If however he *is* convinced that !pFoo is
> > impossible absent any bugs, but argues that if there *are* bugs, the
> > added if statement adds some sort of safety,
>
> I have no strong feelings either way, but might as well agree now while
> we can. So the plan is that asserts are for 100% can never happen
> things. So that would suggest that anything which might fail for
> external reasons is not a candidate for assert.
>
> oslModule hModule= osl_loadModule( foo );
> assert(hModule)
> if (!hModule)
>      throw bar;
>
> is wrong, because foo might not exist if some member of the lunatic
> fringe deleted some .sos out of his install. Or more fairly, his distro
> tried to split up packages into subpackages and mis-categorized one of
> them.

Yes, that's my model, too.  Even if missing/corrupted files that belong 
to the installation are somewhat more on the scale towards "cannot 
happen" than, say, trying to read malformed .odt files, they still 
represent external errors, not logic programming errors.

It would be good if such "impossible to proceed" situations would lead 
to uncaught exceptions (which is IMO acceptable in case of a fucked up 
installation) or clear error messages from within LO ("this 
functionality is unavailable (detailed error message: ...)"), instead of 
LO silently doing nothing (as is so often the case today, e.g., when you 
choose a menu item that triggers functionality that requires Java, and 
you don't have a JVM).

> In which case, we should use SAL_WARN to indicate its an unlikely and
> suspicious event. So do we then consider SAL_WARNs as failures from the
> perspective of e.g. the smoketest where we can argue reasonably that
> we're in a controlled environment and nothing unusual should occur ?

I'm somewhat undecided here.  The smoketest used to fail on OSL_ASSERTs 
(at least partly motivated by the assumption that OSL_ASSERTs represent 
true assertions; even if that assumption was false, the OSL_ASSERTs that 
did fire during smoketest were generally either true assertions that got 
fixed, or of informative nature and degraded to OSL_TRACE, IIRC).  I 
think I effectively removed that with my patch (it still fails for true 
asserts that abort now, of course), but intended to revisit that.  On 
the one hand, your rationale is probably true that SAL_WARNs probably 
always indicate severe enough problems that they would not normally fire 
in the controlled environment of smoketest.  So, from a practical 
standpoint, failing smoketest on failed SAL_WARNs would be right.  On 
the other hand, there might be SAL_WARNs that legitimately fire during 
smoketest (a trivial example would be if we purposefully tested illegal 
input during smoketest), so from a theoretical standpoint failing 
smoketest on failed SAL_WARNs would be wrong.  But I think I'll stick 
with the practical standpoint for now (if only since its easier to 
eventually change from a failing smoketest to one that ignores SAL_WARNs 
than vice versa).

Stephan