On Mon, 2014-10-20 at 15:59 +0200, Ross Gammon wrote: > Hi All, > > I know everyone is busy with the Jessie Release Freeze, but I would be > grateful if somebody could take a look at abcmidi (and sponsor if > happy). Abcmidi has been sitting unloved for a while now (since 2007). > It would be great to get the latest version into Jessie.
Hi, Here's a review (I'm not a DD so can't sponsor you however). General * There is a new upstream version (16th October 2014). * #764998 abcmidi: binary-without-manpage usr/bin/abcmatch Obviously you know this, but it would be good if a manpage was added. * The file "/usr/share/doc/abcmidi/VERSION" seems redundant and can probably be removed. debian/copyright * You don't need to list abc.h, sizes.h, structs.h manually in the first section since they're already included when you say "Files: *". * There seems to be some confusion about whether the code is GPL-2 or GPL-2+. Are you sure what you've put is correct? I see files with no copyright headers but nothing with "GPL 2 only" in them. * You don't need to repeat the GPL header lots of times. I'd also be tempted to merge all the GPL sections together and just have a large "Copyright:" block. debian/rules * I don't think you need to use autotools-dev in this package (I don't know a huge amount about this though). * The clean target doesn't work because you disabled it. This is a violation of debian policy (4.9) "clean (required): This must undo any effects that the build and binary targets may have had" debian/patches: * Make sure you send these patches upstream (sorry if you've already done this - they're not in the new version though). * hardening.patch: Only LDFLAGS should be passed during the link stage. Remove your CFLAGS and CPPFLAGS additions. Build There are lots of bad warnings printed when building this Examples: * parseabc.c:1701:3: warning: format ‘%s’ expects argument of type ‘char *’, but argument 3 has type ‘char **’ [-Wformat=] success = sscanf (s, "%%abc-version %s", &abcversion); /* [SS] 2014-08-11 */ Isn't this a buffer overflow?! * toabc.c:1490:8: warning: iteration 7u invokes undefined behavior [-Waggressive-loop-optimizations] semi = convertnote[i]; It's not too difficult to use these to make abc2midi segfault - please try and fix them if you have time. James _______________________________________________ pkg-multimedia-maintainers mailing list pkg-multimedia-maintainers@lists.alioth.debian.org http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/pkg-multimedia-maintainers
