Skip to content

Commit

Permalink
Merge pull request #21 from valeriyvan/uninit
Browse files Browse the repository at this point in the history
 Eliminates execution path with using uninitialised pointer.

This is a tricky case to parse.  The code is correct, but not something the compiler would be able to figure out automatically.  I will adjust after pulling to make the token NULL when it is created and then add an explicit check for nullness later on which should make automated code analysis happy.   

Also note that humlib is tab indented (I will edit to add them after merging).

Background:

There are two types of line types in HumdrumLine class: lines with "spines" and lines without "spines".  Lines with spines contain one or data fields (that are separated by tabs when there is more than one of them).  Lines without spines contain only one field, and are not considered data.  There are only two possible non-spine line types: those that start with `!!` characters, and also an empty line containing no characters.   All other line types are "spined", meaning they may have internal structure, similar to tracks in a MIDI file.

The code:

```cpp
      if (hasSpines()) {
         line->appendToken(token);
      }
```

was causing a problem since `token` could theoretically be undefined.   However, the previous lines:

```cpp
      } else if (hasSpines()) {
         token = new HumdrumToken("55");
         empty = "!z";
      }
```

will guarantee that `token` is initialized to `55` if such a case were to occur.   

The code:

```cpp
   if (recip) {
      if (isNoteSlice()) {
...
      } else if (isClefSlice()) {
...
      } else if (isMeasureSlice()) {
...
      } else if (isInterpretationSlice()) {
...
      } else if (isGraceSlice()) {
...
      } else if (hasSpines()) {
         token = new HumdrumToken("55");
         empty = "!z";
      }
      if (hasSpines()) {
         line->appendToken(token);
      }
   }
```

Will only append the token if the line has spines, and the token is guaranteed to be allocated properly to "55" in the case of a strange problem.  The tricky/subtle thing is that all of the other cases in the if-statement are expected spine types. 

By adding the extra case to initialize token at the end of the first if/else if statement, a memory leak would occur when a non-spined HumdrumLine is processed:  The token would be allocated, but since the line is not spined, the token would not be stored and the allocated space would never be freed.
  • Loading branch information
craigsapp authored Nov 26, 2019
2 parents 642b185 + 548302b commit 27b2298
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 6 deletions.
2 changes: 1 addition & 1 deletion include/humlib.h
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
//
// Programmer: Craig Stuart Sapp <[email protected]>
// Creation Date: Sat Aug 8 12:24:49 PDT 2015
// Last Modified: Thu Nov 21 16:17:06 WET 2019
// Last Modified: Mon Nov 25 20:49:29 CET 2019
// Filename: humlib.h
// URL: https://github.com/craigsapp/humlib/blob/master/include/humlib.h
// Syntax: C++11
Expand Down
6 changes: 4 additions & 2 deletions src/GridSlice.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ bool GridSlice::isDataSlice(void) {
//

void GridSlice::transferTokens(HumdrumFile& outfile, bool recip) {
HTp token;
HTp token = NULL;
HumdrumLine* line = new HumdrumLine;
GridVoice* voice;
string empty = ".";
Expand Down Expand Up @@ -302,7 +302,9 @@ void GridSlice::transferTokens(HumdrumFile& outfile, bool recip) {
} else if (hasSpines()) {
token = new HumdrumToken("55");
empty = "!z";
}
} else if (!token) {
token = new HumdrumToken();
}
if (hasSpines()) {
line->appendToken(token);
}
Expand Down
8 changes: 5 additions & 3 deletions src/humlib.cpp
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
//
// Programmer: Craig Stuart Sapp <[email protected]>
// Creation Date: Sat Aug 8 12:24:49 PDT 2015
// Last Modified: Thu Nov 21 16:17:06 WET 2019
// Last Modified: Mon Nov 25 20:49:29 CET 2019
// Filename: /include/humlib.cpp
// URL: https://github.com/craigsapp/humlib/blob/master/src/humlib.cpp
// Syntax: C++11
Expand Down Expand Up @@ -5721,7 +5721,7 @@ bool GridSlice::isDataSlice(void) {
//

void GridSlice::transferTokens(HumdrumFile& outfile, bool recip) {
HTp token;
HTp token = NULL;
HumdrumLine* line = new HumdrumLine;
GridVoice* voice;
string empty = ".";
Expand Down Expand Up @@ -5765,7 +5765,9 @@ void GridSlice::transferTokens(HumdrumFile& outfile, bool recip) {
} else if (hasSpines()) {
token = new HumdrumToken("55");
empty = "!z";
}
} else if (!token) {
token = new HumdrumToken();
}
if (hasSpines()) {
line->appendToken(token);
}
Expand Down

0 comments on commit 27b2298

Please sign in to comment.