Re: Another patch

[Dan Eble]

On May 10, 2020, at 08:11, address@hidden wrote:
> 
> I did replace all implicit casts to an int by a inline function, checking if
> the value is valid, and then casting  to int.
> 
> Together with my previous patch now all but one compiler warnings are
> solved.

Jaap,

I love the fact that you are taking the initiative to work on these.  I haven't 
reviewed them thoroughly, but my first comment is a note of caution.  A couple 
of warnings, like the printf one, are just simple oversights; however, many of 
the warnings that remain in LilyPond are the tip of an iceberg.

There was a discussion on the developer list about simply casting to silence 
warnings, but more developers were in favor of leaving the warnings in until 
someone is motivated to fix them properly.

I commend you that your work is not just simply casting, but in at least some 
cases, it still doesn't appear to be going to the depth these issues deserve.  
For example, the number of tracks in a MIDI file is a 16-bit number[1], so 
clipping to INT_MAX isn't quite right.  (I've got an old branch where I've 
fixed the header generation to use uint16_t, but it's still missing code to 
warn properly when the number of tracks needs to be clipped.  I'll try to 
rebase that to save you the work.)

I encourage you to sort the simple and obvious fixes (like the printf one) into 
one commit, and the other cases into their own commits.  It will limit the 
consequences if we later have to revert a change because of a subtle bug that 
escaped regression testing.

Regards,
— 
Dan

[1] 
http://www.music.mcgill.ca/~ich/classes/mumt306/StandardMIDIfileformat.html#BM2_1