RE: Another patch

[lilypond]

Dan, 

As you already did see, I did not a simple cast, but instead my solution is 
raising an error in those cases where a simple cast would change the value 
tested.
However, I will follow your suggestion, and break the patch in parts.
I also will pay attention to your particular example, as I think this is the 
only case of the now solved warnings where the int in a next function is casted 
to a smaller type.

Jaap de Wolff

> -----Oorspronkelijk bericht-----
> Van: Dan Eble <address@hidden>
> Verzonden: Sunday, May 10, 2020 4:31 PM
> Aan: address@hidden
> CC: address@hidden
> Onderwerp: Re: Another patch
> 
> 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.ht
> ml#BM2_1