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

Hi Tor,

Thanks for your reply.

On Tue, 18 Jan 2011 01:49:01 -0700, "Tor Lillqvist" <tlillqvist@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