[Libreoffice] [PATCH] possible null-dereferencing found by cppcheck

[Takeshi Abe]

Hi Tor,

Thanks for your reply.

On Tue, 18 Jan 2011 01:49:01 -0700, "Tor Lillqvist" <tlillqvist at novell.com> wrote:
> 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?)
Caolán has fixed the code around in a way to ensure pPV always initialized.

> 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.
I would like to clarify if DBGUTIL has no power to detect and tell something
wrong occurred in a production build, because it is off then, isn't it?

> 
> 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...
I see your point on using assersion, but yes, such a message can be confusing
for some users.

Cheers,
-- Takeshi Abe