Challenge 5 – Dead, unused, predetermined and suspicious code
The package Conductor defines an automated conductor for an orchestra, which is updated both periodically every few micro-seconds (entry Advance of task Concert) and each time a sensor detects that a musician plays a new note (entry Start_Of_Note of task Concert).
Have a look at the following code and see if you can spot the errors. Once you think you’ve got them all, click on the “Go CodePeer” button to see how well you matched up to CodePeer’s comprehensive and rapid analysis of the same code.
Error line 97: “high: conditional check raises exception here: checking that Part >= 1”
Here’s how CodePeer helps you coming to this conclusion: Local variable Part is initialized to zero, whereas array Orchestra starts at index one, hence the mismatch. In fact, the code here is dead, as the test for the loop will fail at the very first iteration. CodePeer is reporting that an error would occur if the execution was able to reach this program point.
Error line 114: “high: conditional check raises exception here: checking that Part >= 1 and Part <= num_parts.all”
Here’s how CodePeer helps you coming to this conclusion: Local variable Part is initialized to zero, whereas array Orchestra starts at index one, hence the mismatch. Contrary to the previous error, the code is not dead here, due to the different form of the loop test. CodePeer reports the same error on the following line 115 when accessing field Dur.
Error line 125: “warning: suspicious constant operation (min(…))/96 always evaluates to 0”
Here’s how CodePeer helps you coming to this conclusion: CodePeer computes a range of Score.Duration’First .. Score.Duration’Last – 1 for local variable Min from its initialization on line 122. Then, dividing Min by Score.Duration’Last can only result in value zero, which is clearly suspicious for an operation on non-constant operands. The problem stems from the inversion of the conversion to fixed-point type Position and the division on line 125, so that the division occurs in integers here instead of fixed-points.
Error line 156: “medium: condition predetermined because (Dur <= Score.Duration'last) is always true”
Here’s how CodePeer helps you coming to this conclusion: Local variable Dur is of type Score.Duration, hence it cannot exceed Score.Duration’Last. The problem here is that the programmer used Dur where Cumul_Dur was expected.
Error line 167: “warning: unused assignment into Dur”
Here’s how CodePeer helps you coming to this conclusion: Dur is a local variable, hence assigning to it is only useful if its value is read afterwards, which is not the case here. Instead of defining Dur as a local variable on line 143, it should be defined as a renaming like Pos on line 140.
--------------------
-- file score.ads --
--------------------
package Score is
type Pitch is new Natural range 0 .. 127;
-- MIDI note in the MIDI Tuning Standard, only representing semitones here
type Long_Duration is new Natural;
subtype Duration is Long_Duration range 0 .. 96;
-- Fraction of a measure, 96 representing the complete measure, which
-- allows representing from thirty-second note to whole note for measures
-- with one to four beats
type Position is delta 10.0**(-2) range 0.0 .. 10_000.0;
-- Distance from the start of the score in number of measures
type Instrument is
(Flute, Recorder, Violin, Trumpet, Clarinet, Oboe, Cornet, Flugelhorn,
Piccolo_Trumpet, Piccolo, Alto_Saxophone, Alto_Flute, Viola, Horn,
Alto_Horn, Trombone, Tenor_Saxophone, Bass_Trumpet, Bassoon,
English_Horn, Baritone_Saxophone, Baritone_Horn, Bass_Clarinet, Cello,
Euphonium, Bass_Trombone, Contrabassoon, Bass_Saxophone, Double_Bass,
Tuba, Contrabass_Saxophone, Contrabass_Bugle);
type Interpreter is (First, Second, Third, Fourth, Fifth);
procedure Get_Note
(Inter : Interpreter;
Instr : Instrument;
Pos : Position;
Fwd : Long_Duration;
Pit : out Pitch;
Dur : out Duration);
-- Return the Pitch and Duration of the note starting forward by Fwd from
-- position Pos for the Interpreter of the Instrument
end Score;
------------------------
-- file conductor.ads --
------------------------
with Score; use Score;
package Conductor is
type Positions is array (Positive range <>) of Position;
type Durations is array (Positive range <>) of Score.Duration;
type Part (Max_Players : Positive) is record
Inter : Interpreter; -- Interpreter for this part
Instr : Instrument; -- Instrument for this part
Num_Players : Positive; -- Number of players in 1 .. Max_Players
Pos : Positions (1 .. Max_Players);
-- Current position of each player in the score
Dur : Durations (1 .. Max_Players);
-- Remaining duration of the current note for each player
end record;
type Parts is array (Positive range <>) of Part (5);
task type Concert (Num_Parts : Positive) is
-- Initialize the parts to be played
entry Initialize (Initial_Parts : Parts);
-- Signal that a player starts a note
entry Start_Of_Note
(Index : Positive; -- Index of the Part being played in the array
Num : Positive; -- Number of the player in the Part
Pit : Pitch); -- Pitch of the note
-- Advance the position of all players by a Step, which is expected to
-- be much smaller than the interval between two consecutive notes
entry Advance (Step : Score.Duration);
-- Start of the concert
entry Play;
end Concert;
end Conductor;
------------------------
-- file conductor.adb --
------------------------
package body Conductor is
task body Concert is
Orchestra : Parts (1 .. Num_Parts);
begin
accept Initialize (Initial_Parts : Parts)
do
declare
Part : Integer := 0;
begin
while Part in Orchestra'Range loop
Orchestra (Part) := Initial_Parts (Part);
Part := Part + 1;
end loop;
end;
end Initialize;
accept Play; -- Start the concert
loop
select
accept Advance (Step : Score.Duration)
do
declare
Part : Integer := 0;
begin
while Part < Num_Parts loop
declare
Pos : Positions renames Orchestra (Part).Pos;
Dur : Durations renames Orchestra (Part).Dur;
begin
for Index in Pos'Range loop
declare
-- Do not allow to advance beyond a new note as
-- it is the role of Start_Of_Note to do so
Min : constant Score.Duration :=
Score.Duration'Min (Step, Dur (Index) - 1);
begin
Pos (Index) := Pos (Index) +
Position (Min / Score.Duration'Last);
end;
end loop;
end;
Part := Part + 1;
end loop;
end;
end;
or
accept Start_Of_Note
(Index : Positive;
Num : Positive;
Pit : Pitch)
do
declare
Pos : Position renames Orchestra (Index).Pos (Num);
Inter : Interpreter := Orchestra (Index).Inter;
Instr : Instrument := Orchestra (Index).Instr;
Dur : Score.Duration := Orchestra (Index).Dur (Num);
Expect_Pit : Pitch;
Expect_Dur : Score.Duration;
Cumul_Dur : Long_Duration := Dur;
begin
-- Look for the next note to be played
Get_Note (Inter, Instr, Pos, Dur, Expect_Pit, Expect_Dur);
-- If this note's pitch does not match, suppose that either
-- the player or the sensor missed a note and continue
while Pit /= Expect_Pit and then
-- Look for a note at the appropriate pitch
Dur <= Score.Duration'Last
-- If it cannot be found one measure in advance, give up
loop
Cumul_Dur := Cumul_Dur + Expect_Dur;
Get_Note
(Inter, Instr, Pos, Cumul_Dur, Expect_Pit, Expect_Dur);
end loop;
if Pit = Expect_Pit then
-- A matching note was found, so update the state
Pos := Pos + Position (Cumul_Dur / Score.Duration'Last);
Dur := Expect_Dur;
end if;
end;
end Start_Of_Note;
end select;
end loop;
end Concert;
end Conductor;