Bug#765009: Subject: RFS: abcmidi/20140928-1 [ITA]

[Ross Gammon]

Yes - thanks to both James & Tobias for your help with this!

I was working through the issues one by one, and was planning to
update the RFS bug with my status today :-)

The manpage is done, and just needs checking. I have already been in
touch with upstream about the 3 "Mayhem" bugs, and will forward the
manpage & the other patches. I will also discuss the other warnings &
build issues.

The upstream author (Seymour) actually uses Debian, so he is extra
helpfull!!

Regards,

Ross

On 10/25/2014 01:18 AM, Tobias Frost wrote:
> Many thanks James! Valid points
> 
> Ross, please also consider those comments. Especially please fix
> the build system. I missed that during my review, sorry, but I will
> file a bug for that.
> 
> (Also, please send your patches upstream.)
> 
> -- tobi
> 
> 
> Am Montag, den 20.10.2014, 21:59 +0100 schrieb James Cowgill:
>> 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.
> 
> Also ÁUTHORS should not be installed.
> 
> 
>> 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
>> 
>> 
>