'Review Request 127658: Fix Coverty Scan ID 1342595 - Dereference before null check'

[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-edu-devel
Subject:    Review Request 127658: Fix Coverty Scan ID 1342595 - Dereference before null check
From:       Hartmut Riesenbeck <hartmut.riesenbeck () gmx ! de>
Date:       2016-04-15 20:47:48
Message-ID: 20160415204748.15182.80131 () mimi ! kde ! org
[Download RAW message or body]

--===============5094763918110665034==
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127658/
-----------------------------------------------------------

Review request for KDE Edu.

Repository: parley

Description
-------

Refactoring of class Collection. The pointer access before null check
is located in the close() method wich is called in the destructor.
Further invetigation showed that this call is useless because the
KEduVocDocument is opened read only (see LibKEduVocDocument::close()).
The close method call was removed.

While further refactoring more unused fuctionality was removed.

I'am not shure if this refactoring is too radical. Please correct me if I deleted too \
much.

The Collection class is only used for counting the vocabulary expression states in \
the files shown on the dashboard page. So I removed all the functionality which is \
not used anywhere else in the source code. This makes the Collection class \
lightweigt, more simple and easy to understand.

+ Since the only Collection objects are constructed in dashboard.cpp the second \
constructor with pointer argument was removed. + Now KEduVocDocument files are only \
opened readonly. This makes closing, auto backup and the change signals useless. + \
Also the fetch grammar functionallity is not used.

The TODO in collection.cpp line 134 seems to me already be done, so I removed it. \
Please correct me if I'm wrong.

Diffs
-----

  src/collection/collection.h 4e91603eea5a33af781104fcb856f4483b1ab5b8 
  src/collection/collection.cpp 004d85fe411c882b541d0e1f8ecbfdf213d528c3 
  src/dashboard/dashboard.cpp 4e60b10a1501a20d0f4741344035a5bc9b400d14 

Diff: https://git.reviewboard.kde.org/r/127658/diff/

Testing
-------

Thanks,

Hartmut Riesenbeck

--===============5094763918110665034==
MIME-Version: 1.0
Content-Type: text/html; charset="utf-8"
Content-Transfer-Encoding: 7bit

<html>
 <body>
  <div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
   <table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 \
solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">  \
<tr>  <td>
      This is an automatically generated e-mail. To reply, visit:
      <a href="https://git.reviewboard.kde.org/r/127658/">https://git.reviewboard.kde.org/r/127658/</a>
  </td>
    </tr>
   </table>
   <br />

<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: \
1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; \
-webkit-border-radius: 6px;">  <tr>
  <td>

<div>Review request for KDE Edu.</div>
<div>By Hartmut Riesenbeck.</div>

<div style="margin-top: 1.5em;">
 <b style="color: #575012; font-size: 10pt;">Repository: </b>
parley
</div>

<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
 <table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" \
style="border: 1px solid #b8b5a0">  <tr>
  <td>
   <pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">Refactoring of class Collection. The pointer access \
before null check is located in the close() method wich is called in the destructor.
Further invetigation showed that this call is useless because the
KEduVocDocument is opened read only (see LibKEduVocDocument::close()).
The close method call was removed.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">While further refactoring more unused fuctionality was \
removed.</p> <p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">I'am not shure if this refactoring is too radical. \
Please correct me if I deleted too much.</p> <p style="padding: 0;text-rendering: \
inherit;margin: 0;line-height: inherit;white-space: inherit;">The Collection class is \
only used for counting the vocabulary expression states in the files shown on the \
dashboard page. So I removed all the functionality which is not used anywhere else in \
the source code. This makes the Collection class lightweigt, more simple and easy to \
understand.</p> <ul style="padding: 0;text-rendering: inherit;margin: 0 0 0 \
1em;line-height: inherit;white-space: normal;"> <li style="padding: 0;text-rendering: \
inherit;margin: 0;line-height: inherit;white-space: normal;">Since the only \
Collection objects are constructed in dashboard.cpp the second constructor with \
pointer argument was removed.</li> <li style="padding: 0;text-rendering: \
inherit;margin: 0;line-height: inherit;white-space: normal;">Now KEduVocDocument \
files are only opened readonly. This makes closing, auto backup and the change \
signals useless.</li> <li style="padding: 0;text-rendering: inherit;margin: \
0;line-height: inherit;white-space: normal;">Also the fetch grammar functionallity is \
not used.</li> </ul>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">The TODO in collection.cpp line 134 seems to me \
already be done, so I removed it. Please correct me if I'm wrong.</p></pre>  </td>
 </tr>
</table>

<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">

 <li>src/collection/collection.h <span style="color: \
grey">(4e91603eea5a33af781104fcb856f4483b1ab5b8)</span></li>

 <li>src/collection/collection.cpp <span style="color: \
grey">(004d85fe411c882b541d0e1f8ecbfdf213d528c3)</span></li>

 <li>src/dashboard/dashboard.cpp <span style="color: \
grey">(4e60b10a1501a20d0f4741344035a5bc9b400d14)</span></li>

</ul>

<p><a href="https://git.reviewboard.kde.org/r/127658/diff/" style="margin-left: \
3em;">View Diff</a></p>

  </td>
 </tr>
</table>

  </div>
 </body>
</html>

--===============5094763918110665034==--

[Attachment #3 (text/plain)]

_______________________________________________
kde-edu mailing list
kde-edu@mail.kde.org
https://mail.kde.org/mailman/listinfo/kde-edu

[prev in list] [next in list] [prev in thread] [next in thread] 