verdammelt / tnef

tnef
GNU General Public License v2.0
58 stars 21 forks source link

Fix Fedora FTBFS with -Werror=format-security enabled. #8

Closed dtimms closed 10 years ago

dtimms commented 10 years ago

During the Fedora 21 development cycle, all packages have been rebuilt with the compile option: -Werror=format-security, see https://fedoraproject.org/wiki/Format-Security-FAQ The purpose is to cause compilation failures when certain detectable string format constructions have been used. The Fedora bug is: https://bugzilla.redhat.com/show_bug.cgi?id=1037361 and a patch against v1.4.9 is attached: https://bugzilla.redhat.com/attachment.cgi?id=908703&action=diff

This patch is slightly modified to suit the removal of the COPYRIGHT string.

ileGITimo commented 10 years ago

Latest tnef is 1.4.11 which fixes a problem for multi-value fields. The patch above for 1.4.9 works fine agains the master/1.4.11. Shouldn't F21 include this latest stuff, otherwise I'll have to keep compiling it myself.

dtimms commented 10 years ago

ileGITimo: I'm package maintainer for Fedora tnef. I have updated to 1.4.11, but on Fedora 21 I need to apply a patch to improve format function security to work with gcc -Werror=format-security. I'm asking the patch to be upstreamed into git. I'm new to github, but it seems the "way" is to git clone, make changes and then do a pull request. It seems this gives the master developer the easiest way to accept an update. I'm happy to send the patch if that is preferred ?

dtimms commented 10 years ago

The 1.4.9 patch tries to patch: +static const char COPYRIGHTS[] = \ , but this line no longer exists in the source code. On Fedora patches are appiled with fuzz=0, and hence doesn't apply with 1.4.11. That's why I've updated the patch / made the pull request.

verdammelt commented 10 years ago

I see nothing particularly wrong with the pull request, except for the second commit you added 5a23394. That breaks the build, looks like a simple mistake. Remove that so that build passes again and I'll merge this in.

verdammelt commented 10 years ago

Merged this by hand.