On Sat, 05 Sep 2009 18:30:13 +0200, Colomban wrote:
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
Hi,
Thomas Martitz a écrit :
My patch was buggy. I don't know how I didn't notice it while testing. This should fix it http://pastie.org/606912
Nothing important, but I wonder why you test for (!reload) as you already check for (reload)? The check at the right of a || switch will be tested only if the left one is not true, then if the right check happens, then reload is necessarily false, isn't it? But it just seems strange and complicated to me, not a problem anyway.
Same here.
Before the patch we only checked for "if (reload)". The function documentation already says: if reload is set, the doc argument has to be non-NULL, if reload is not set, doc is NULL. So, the new, additional check is somewhat unnecessary. It could be argumented as an additional sanity check but generally, document_open_file_full() is an internal function and so if the above conditions of the input argumens are not fullfilled, I would consider this as a bug in the calling code (still sanity checks are good, no doubt).
Regards, Enrico