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 <d...@faithful.be> > Verzonden: Sunday, May 10, 2020 4:31 PM > Aan: lilyp...@de-wolff.org > CC: lilypond-devel@gnu.org > Onderwerp: Re: Another patch > > On May 10, 2020, at 08:11, lilyp...@de-wolff.org 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
