[Libreoffice] [PATCH] possible null-dereferencing found by cppcheck
Tor Lillqvist
tlillqvist at novell.com
Tue Jan 18 00:49:01 PST 2011
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
More information about the LibreOffice
mailing list