Re: Fishy assignment in editdoc (editeng) ?

2019-05-27 Thread Malte Timmermann

For editdoc.cxx:

== op compares the individual members of the font if the impl pointer is
not equal. If all members are the same, the strange assignment makes
sure that both also point to the same internal instance of the font. If
you want to avoid that strange assignment, you would need to check in
every if statement above whether or not the new value differs from the
old value before making an assignment.

Best regards,
Malte.


On 26.05.2019 21:24, julien2412 wrote:

Hello,

Taking a look to the new reports generated by Cppcheck, I noticed
"duplicateConditionalAssign" and specifically this code:
2028 // If comparing the entire font, or if checking before each
alteration
2029 // whether the value changes, remains relatively the same thing.
2030 // So possible one MakeUniqFont more in the font, but as a
result a quicker
2031 // abortion of the query, or one must each time check bChanged.
2032 if ( rFont == aPrevFont  )
2033 rFont = aPrevFont;  // => The same ImpPointer for
IsSameInstance

See
https://opengrok.libreoffice.org/xref/core/editeng/source/editeng/editdoc.cxx?r=40d25921#2028

Another similar place:
xmlsecurity/source/xmlsec/nss/securityenvironment_nssimpl.cxx
 800 if( xcert == nullptr ) {
 801 xcert = nullptr ;  <-- whereas in other parts of the
same file, we see a throw instruction
 802 } else {

Any thoughts?

Julien




--
Sent from: 
http://document-foundation-mail-archive.969070.n3.nabble.com/Dev-f1639786.html
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice

___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice

Re: Fishy assignment in editdoc (editeng) ?

2019-05-27 Thread Stephan Bergmann

On 26/05/2019 21:24, julien2412 wrote:

Taking a look to the new reports generated by Cppcheck, I noticed
"duplicateConditionalAssign" and specifically this code:
2028 // If comparing the entire font, or if checking before each
alteration
2029 // whether the value changes, remains relatively the same thing.
2030 // So possible one MakeUniqFont more in the font, but as a
result a quicker
2031 // abortion of the query, or one must each time check bChanged.
2032 if ( rFont == aPrevFont  )
2033 rFont = aPrevFont;  // => The same ImpPointer for
IsSameInstance

See
https://opengrok.libreoffice.org/xref/core/editeng/source/editeng/editdoc.cxx?r=40d25921#2028


From a quick look, this looks plausible (if the two fonts compare 
equal, but have potentially different innards, make them share a single 
set of innards).



Another similar place:
xmlsecurity/source/xmlsec/nss/securityenvironment_nssimpl.cxx
 800 if( xcert == nullptr ) {
 801 xcert = nullptr ;  <-- whereas in other parts of the
same file, we see a throw instruction
 802 } else {


That was apparently nonsense,  
"operator new doesn't return null anyway".  Thanks for pointing it out.

___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice

Fishy assignment in editdoc (editeng) ?

2019-05-26 Thread julien2412
Hello,

Taking a look to the new reports generated by Cppcheck, I noticed
"duplicateConditionalAssign" and specifically this code:
   2028 // If comparing the entire font, or if checking before each
alteration
   2029 // whether the value changes, remains relatively the same thing.
   2030 // So possible one MakeUniqFont more in the font, but as a
result a quicker
   2031 // abortion of the query, or one must each time check bChanged.
   2032 if ( rFont == aPrevFont  )
   2033 rFont = aPrevFont;  // => The same ImpPointer for
IsSameInstance

See
https://opengrok.libreoffice.org/xref/core/editeng/source/editeng/editdoc.cxx?r=40d25921#2028

Another similar place:
xmlsecurity/source/xmlsec/nss/securityenvironment_nssimpl.cxx
800 if( xcert == nullptr ) {
801 xcert = nullptr ;  <-- whereas in other parts of the
same file, we see a throw instruction
802 } else {

Any thoughts?

Julien




--
Sent from: 
http://document-foundation-mail-archive.969070.n3.nabble.com/Dev-f1639786.html
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice