Thanks for the patch. The code in that method, sd/source/ui/func/fusnapln.cxx: 
FuSnapLine::DoExecute( SfxRequest& rReq ), is indeed interesting. In theory 
there are code paths through it that lead to a NULL pPV (and pArgs) pointer 
de-references, as cppcheck correctly notices.  (But clearly, assuming that the 
method gets called at all, those code paths are not taken, or it would crash. 
Or does that happen, maybe that is a rare but possible situation, and nobody 
has reported the crash?)

Anyway, I think that instead of just surrounding a block of code with if 
(pPV!=NULL), which then no doubt means something else breaks, we should use an 
assertion so that we get a clear error message in those (rare) situations where 
the NULL pointer de-reference would happen. Hmm, except that was it so that 
there is no "standard" way in OOo/LibreOffice to get assertions in a non-debug 
production build? I am totally confused by the DBGUTIL etc stuff.

Do we have some kind of message displaying assertion from which the user can 
continue? In that case we should then of course guard against a NULL 
de-reference after displaying the message.

I.e., I would ideally display something like "LibreOffice has found itself in a 
weird situation. It is not crashing, but something might be wrong. You can 
continue your work after pressing OK. Still, to be safe, please save your 
modified documents under new names and quit. Afterwards start LibreOffice 
again, make sure that the saved documents still are correct, and if they are OK 
do save them back as their original names" but I certainly realize that no user 
would understand what we are trying to say...

Or is it our policy too that we should never, ever, display to the user any 
warning message that sounds like admitting we don't know what the code is 
doing...?

--tml


_______________________________________________
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice

Reply via email to