Re: MUMmer patches and Artistic license.
On Sun, Jul 12, 2009 at 10:50:10PM +0900, Charles Plessy wrote: Hi, thank you for the comment. I will stop to apply this patch until a better solution is found. Charles, could you please give an update of the status of mummer package? Kind regards Andreas. -- http://fam-tille.de -- To UNSUBSCRIBE, email to debian-med-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Re: MUMmer patches and Artistic license.
Le Mon, Oct 05, 2009 at 08:41:05AM +0200, Andreas Tille a écrit : On Sun, Jul 12, 2009 at 10:50:10PM +0900, Charles Plessy wrote: Hi, thank you for the comment. I will stop to apply this patch until a better solution is found. Charles, could you please give an update of the status of mummer package? Hi Andreas, Almost everything is done. There is a TODO point left in the changelog about restoring a renaming. Since MUMmer is released under the artistic license, we can not upload without double-checking that programs modified without the green light of Upstream are indeed renamed. The reason I have not done so is that I lost momentum when the upstream corrections for memory management took some time to come. Everybody is welcome to do the upload after clearing this TODO point. I plan to do this after uploading the bwa package that I prepared last week, for which I will ask for a copyright review on the debian-med list. The recent upload of infernal with incomplete copyright summary shows that we would benefit seeking from peer review on this file for new packages. Have a nice day, -- Charles Plessy Debian Med packaging team, http://www.debian.org/devel/debian-med Tsurumi, Kanagawa, Japan -- To UNSUBSCRIBE, email to debian-med-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Re: MUMmer patches and Artistic license.
On Sun, Jul 12, 2009 at 10:50:10PM +0900, Charles Plessy wrote: This patch is a good idea -- fixed length buffers are rarely considered user-friendly. However, it is executed poorly; it's not ready for upstream in its current state. Hi, thank you for the comment. I will stop to apply this patch until a better solution is found. Quite late comment after working down my batch of unread mail: The patch was *not* *only* to avoid fixed length buffers - it was invented to stop mummer from *crashing*. I do not remember the case exactly nor do I find the data any more but I *clearly* remember that we actually had example data which were causing mummer to crash without the patch. Kind regards Andreas. -- http://fam-tille.de -- To UNSUBSCRIBE, email to debian-med-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Re: MUMmer patches and Artistic license.
In 20090728163525.gc31...@an3as.eu, Andreas Tille wrote: On Sun, Jul 12, 2009 at 10:50:10PM +0900, Charles Plessy wrote: This patch is a good idea -- fixed length buffers are rarely considered user-friendly. However, it is executed poorly; it's not ready for upstream in its current state. Hi, thank you for the comment. I will stop to apply this patch until a better solution is found. Quite late comment after working down my batch of unread mail: The patch was *not* *only* to avoid fixed length buffers - it was invented to stop mummer from *crashing*. In that case I recommend leaving the patch applied, opening a bug, and assigning it to me. I can re-write the patch to avoid having statements with side-effects inside assert() calls. I may be able to reply with a corrected patch later today. P.S. Please CC me if you drop debian-mentors, I am not subscribed to debian-med. -- Boyd Stephen Smith Jr. ,= ,-_-. =. b...@iguanasuicide.net ((_/)o o(\_)) ICQ: 514984 YM/AIM: DaTwinkDaddy `-'(. .)`-' http://iguanasuicide.net/\_/ signature.asc Description: This is a digitally signed message part.
Re: MUMmer patches and Artistic license.
In 200907281303.42301@iguanasuicide.net, Boyd Stephen Smith Jr. wrote:
In 20090728163525.gc31...@an3as.eu, Andreas Tille wrote:
On Sun, Jul 12, 2009 at 10:50:10PM +0900, Charles Plessy wrote:
This patch is a good idea -- fixed length buffers are rarely
considered user-friendly. However, it is executed poorly; it's not
ready for upstream in its current state.
Hi, thank you for the comment.
I will stop to apply this patch until a better solution is found.
Quite late comment after working down my batch of unread mail:
The patch was *not* *only* to avoid fixed length buffers - it was
invented to stop mummer from *crashing*.
I may be able to reply with a corrected patch later today.
Attached is my new patch that removes the errors I saw in the original
patch. I welcome any comments on the new patch.
I've also attached a interdiff between the old and new patches for
reference.
In case anyone feels this work is copyrightable[1], I license my
contributions under the same license as the original patch.
P.S. Please CC me if you drop debian-mentors, I am not subscribed to
debian-med.
--
Boyd Stephen Smith Jr. ,= ,-_-. =.
b...@iguanasuicide.net ((_/)o o(\_))
ICQ: 514984 YM/AIM: DaTwinkDaddy `-'(. .)`-'
http://iguanasuicide.net/\_/
[1] I don't.
Description: Dynamically allocates space for the strings, instead allocating a predefined size.
Origin: vendor : Debian
Bug: https://sourceforge.net/tracker/?func=detailaid=1215086group_id=133157atid=726404
Index: ./src/tigr/annotate.cc
===
--- ./src/tigr/annotate.cc
+++ ./src/tigr.orig/annotate.cc
@@ -10,6 +10,7 @@
*/
#include tigrinc.hh
+#include assert.h
#define FIELD_LEN 20
#define MAX_ALIGN 1
@@ -138,19 +139,26 @@ void Show_Alignment (char A [], long in
// Print the alignment between strings A [1 .. M] and B [1 .. N] .
{
- static int D [MAX_ALIGN] [MAX_ALIGN];
- static char Op [MAX_ALIGN] [MAX_ALIGN];
- static char Show_A [2 * MAX_ALIGN];
- static char Show_B [2 * MAX_ALIGN];
+ int **D, *D_buf;
+ char **Op, *Op_buf;
+ char *Show_A;
+ char *Show_B;
int Errors, Tmp;
long int i, j, Ct;
- if (M = MAX_ALIGN || N = MAX_ALIGN)
- {
-printf (\n *** Too long ***\n\n);
-fprintf (Gaps_With_Errors_File, %s %7s\n, Line, -);
-return;
- }
+ assert ( SIZE_MAX / (M+1) = (N+1) ) ;
+ D_buf = (int *) calloc ( (M+1)*(N+1), sizeof(int) ) ;
+ assert ( D_buf ) ;
+ D = D_buf ;
+ Op_buf = (char *) calloc ( (M+1)*(N+1), sizeof(char) ) ;
+ assert ( Op_buf ) ;
+ Op = Op_buf ;
+
+ assert ( SIZE_MAX 1 = (M+1) ) ;
+ Show_A = (char *) calloc ( 2*(M+1) , sizeof(char) ) ;
+ assert ( Show_A ) ;
+ Show_B = (char *) calloc ( 2*(N+1) , sizeof(char) ) ;
+ assert ( Show_B ) ;
D [0] [0] = 0;
Op [0] [0] = 'a';
@@ -229,5 +237,10 @@ void Show_Alignment (char A [], long in
putchar ('\n');
Ct -= WIDTH;
} while (Ct 0);
+
+ free ( D_buf ) ;
+ free ( Op_buf ) ;
+ free ( Show_A ) ;
+ free ( Show_B ) ;
return;
}
diff -u ./src/tigr/annotate.cc ./src/tigr.orig/annotate.cc
--- ./src/tigr/annotate.cc 2007-11-07 21:47:03.0 +0100
+++ ./src/tigr.orig/annotate.cc
@@ -146,12 +146,19 @@
int Errors, Tmp;
long int i, j, Ct;
- assert ( D_buf = (int *) calloc ( (M+1)*(N+1), sizeof(int) ) ) ;
+ assert ( SIZE_MAX / (M+1) = (N+1) ) ;
+ D_buf = (int *) calloc ( (M+1)*(N+1), sizeof(int) ) ;
+ assert ( D_buf ) ;
D = D_buf ;
- assert ( Op_buf = (char *) calloc ( (M+1)*(N+1), sizeof(char) ) ) ;
+ Op_buf = (char *) calloc ( (M+1)*(N+1), sizeof(char) ) ;
+ assert ( Op_buf ) ;
Op = Op_buf ;
- assert ( Show_A = (char *) calloc ( 2*(M+1) , sizeof(char) ) ) ;
- assert ( Show_B = (char *) calloc ( 2*(N+1) , sizeof(char) ) ) ;
+
+ assert ( SIZE_MAX 1 = (M+1) ) ;
+ Show_A = (char *) calloc ( 2*(M+1) , sizeof(char) ) ;
+ assert ( Show_A ) ;
+ Show_B = (char *) calloc ( 2*(N+1) , sizeof(char) ) ;
+ assert ( Show_B ) ;
D [0] [0] = 0;
Op [0] [0] = 'a';
signature.asc
Description: This is a digitally signed message part.
Re: MUMmer patches and Artistic license.
In 20090704120642.gh6...@kunpuu.plessy.org, Charles Plessy wrote: we have a patch in the Debian package mummer for which we lost origin and detailed description. I would like to forward it upstream, but I would prefer to know what it does before ;) Would somebody have a few minutes to throw an eye on it? It is in our SVN, and here is a copy: http://svn.debian.org/wsvn/debian-med/trunk/packages/mummer/trunk/debian/p atches/01sm_src_tigr.diff + assert ( D_buf = (int *) calloc ( (M+1)*(N+1), sizeof(int) ) ) ; + D = D_buf ; + assert ( Op_buf = (char *) calloc ( (M+1)*(N+1), sizeof(char) ) ) ; + Op = Op_buf ; + assert ( Show_A = (char *) calloc ( 2*(M+1) , sizeof(char) ) ) ; + assert ( Show_B = (char *) calloc ( 2*(N+1) , sizeof(char) ) ) ; Four out of these six lines are bugs waiting to happen. If NDEBUG is defined, the calls to calloc will be completely elided. Then you'll have a number of uninitialized pointers that are used here: D [0] [0] = 0; Op [0] [0] = 'a'; This patch is a good idea -- fixed length buffers are rarely considered user-friendly. However, it is executed poorly; it's not ready for upstream in its current state. -- Boyd Stephen Smith Jr. ,= ,-_-. =. b...@iguanasuicide.net ((_/)o o(\_)) ICQ: 514984 YM/AIM: DaTwinkDaddy `-'(. .)`-' http://iguanasuicide.net/\_/ signature.asc Description: This is a digitally signed message part.
Re: MUMmer patches and Artistic license.
Dear Debian mentors,
we have a patch in the Debian package mummer for which we lost origin and
detailed description. I would like to forward it upstream, but I would prefer
to know what it does before ;) Would somebody have a few minutes to throw an
eye on it? It is in our SVN, and here is a copy:
http://svn.debian.org/wsvn/debian-med/trunk/packages/mummer/trunk/debian/patches/01sm_src_tigr.diff
diff -urbN src/tigr.orig/annotate.cc src/tigr/annotate.cc
--- ./src/tigr.orig/annotate.cc 2007-07-13 19:06:58.0 +0200
+++ ./src/tigr/annotate.cc 2007-11-07 21:47:03.0 +0100
@@ -10,6 +10,7 @@
*/
#include tigrinc.hh
+#include assert.h
#define FIELD_LEN 20
#define MAX_ALIGN 1
@@ -138,19 +139,19 @@
// Print the alignment between strings A [1 .. M] and B [1 .. N] .
{
- static int D [MAX_ALIGN] [MAX_ALIGN];
- static char Op [MAX_ALIGN] [MAX_ALIGN];
- static char Show_A [2 * MAX_ALIGN];
- static char Show_B [2 * MAX_ALIGN];
+ int **D, *D_buf;
+ char **Op, *Op_buf;
+ char *Show_A;
+ char *Show_B;
int Errors, Tmp;
long int i, j, Ct;
- if (M = MAX_ALIGN || N = MAX_ALIGN)
- {
-printf (\n *** Too long ***\n\n);
-fprintf (Gaps_With_Errors_File, %s %7s\n, Line, -);
-return;
- }
+ assert ( D_buf = (int *) calloc ( (M+1)*(N+1), sizeof(int) ) ) ;
+ D = D_buf ;
+ assert ( Op_buf = (char *) calloc ( (M+1)*(N+1), sizeof(char) ) ) ;
+ Op = Op_buf ;
+ assert ( Show_A = (char *) calloc ( 2*(M+1) , sizeof(char) ) ) ;
+ assert ( Show_B = (char *) calloc ( 2*(N+1) , sizeof(char) ) ) ;
D [0] [0] = 0;
Op [0] [0] = 'a';
@@ -229,5 +230,10 @@
putchar ('\n');
Ct -= WIDTH;
} while (Ct 0);
+
+ free ( D_buf ) ;
+ free ( Op_buf ) ;
+ free ( Show_A ) ;
+ free ( Show_B ) ;
return;
}
There is a bit of background on our mailing lists, but it is really parcellar…
http://lists.debian.org/debian-med/2007/11/msg00045.html
http://lists.debian.org/debian-devel/2005/06/msg00037.html
The take-home message is of course: DEP3 is good for you!
http://dep.debian.net/deps/dep3/
Have a nice day,
--
Charles Plessy
Debian Med packaging team,
http://www.debian.org/devel/debian-med
Tsurumi, Kanagawa, Japan
--
To UNSUBSCRIBE, email to debian-med-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Re: MUMmer patches and Artistic license.
Hi Charles! On Sat, Jul 4, 2009 at 9:06 AM, Charles Plessyple...@debian.org wrote: we have a patch in the Debian package mummer for which we lost origin and detailed description. I would like to forward it upstream, but I would prefer to know what it does before ;) Would somebody have a few minutes to throw an eye on it? It is in our SVN, and here is a copy: It will dynamically allocate space for the strings, instead allocating a predefined size. Best regards, Nelson -- To UNSUBSCRIBE, email to debian-med-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Re: MUMmer patches and Artistic license.
On Thu, Jul 02, 2009 at 01:47:12PM +0900, Charles Plessy wrote: Hi Andreas and Steffen, Taking a new upstream release as an opportunity, I have made a lot of modifications to the mummer package. The current version in Debian relies heavily on patches that are not documented and not always suitable for upstream. We should reduce the patches which are not suitable for upstream to a bare minimum which is really needed. I converted the pacakge from CDBS to debhelper in order to be able to drop most of the patches. Nice to see that debhelper 7 also enables quite compact rules files. (I should have RTFMed since a long time ...) The first question I would like to ask is the function of the changes made to the C++ sources of the ???annotate???. http://patch-tracking.debian.net/patch/series/view/mummer/3.20-3/01sm_src_tigr.diff See http://lists.debian.org/debian-med/2007/11/msg00045.html I am not sure if that was the purpose, but I noticed that ???annotate??? was renamed ???mummer-annotate???, which makes us compliant with the Artistic license under which it is released. The reason was not the license but a name space polution. Imagemagick has a tool called annotate. We should teach upstream not to use generic names! But in addition some C shell programs were replaced by Bourne shell counterparts, without being renamed. What would you recommend? Drop the changes and ship the C shell versions, or forward the Bourne versions upstream and ask permission for using them? I would try to avoid artificial C shell dependency. If teaching upstream is no option I would vote for a rename. But this is just my personal opinion and I would not heavily insist on it. So feel free to decide what might be the best option. Thanks for working on this Andreas. -- http://fam-tille.de -- To UNSUBSCRIBE, email to debian-med-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Re: MUMmer patches and Artistic license.
Le Thu, Jul 02, 2009 at 09:20:47AM +0200, Andreas Tille a écrit : On Thu, Jul 02, 2009 at 01:47:12PM +0900, Charles Plessy wrote: http://patch-tracking.debian.net/patch/series/view/mummer/3.20-3/01sm_src_tigr.diff See http://lists.debian.org/debian-med/2007/11/msg00045.html Hi Andreas, I tried to google further and found no explication about the buffer overflow that this patch is preventing. Do you have something useful in your mailbox? I would like to document it better before forwarding it. At the same time, I will also forward the Bourne shell replacements and wait a bit for the answer. The new upstream release does not bring new functionalities and can wait until Debian switches to GCC 4.4. Have a nice day, -- Charles -- To UNSUBSCRIBE, email to debian-med-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Re: MUMmer patches and Artistic license.
On Thu, Jul 02, 2009 at 07:11:13PM +0900, Charles Plessy wrote: http://lists.debian.org/debian-med/2007/11/msg00045.html Hi Andreas, I tried to google further and found no explication about the buffer overflow that this patch is preventing. Do you have something useful in your mailbox? I would like to document it better before forwarding it. I tried to grep old mails and found another issue with annotate: http://lists.debian.org/debian-devel/2005/06/msg00037.html + following mails in thread But this is probably not helpful. If I remember right we had some test data sets which caused the problem but this knowledge is probably lost in digital space. Steffen, do you keep any record of this? At the same time, I will also forward the Bourne shell replacements and wait a bit for the answer. The new upstream release does not bring new functionalities and can wait until Debian switches to GCC 4.4. I have also some German communication with one of the authors Stefan Kurtz ku...@zbh.uni-hamburg.de where I sended the patches to - but this also brings not more light into the issue. Kind regards Andreas. -- http://fam-tille.de -- To UNSUBSCRIBE, email to debian-med-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
MUMmer patches and Artistic license.
Hi Andreas and Steffen, Taking a new upstream release as an opportunity, I have made a lot of modifications to the mummer package. The current version in Debian relies heavily on patches that are not documented and not always suitable for upstream. I converted the pacakge from CDBS to debhelper in order to be able to drop most of the patches. The first question I would like to ask is the function of the changes made to the C++ sources of the ‘annotate’. http://patch-tracking.debian.net/patch/series/view/mummer/3.20-3/01sm_src_tigr.diff I am not sure if that was the purpose, but I noticed that ‘annotate’ was renamed ‘mummer-annotate’, which makes us compliant with the Artistic license under which it is released. But in addition some C shell programs were replaced by Bourne shell counterparts, without being renamed. What would you recommend? Drop the changes and ship the C shell versions, or forward the Bourne versions upstream and ask permission for using them? Have a nice day, -- Charles Plessy Debian Med packaging team, http://www.debian.org/devel/debian-med Tsurumi, Kanagawa, Japan -- To UNSUBSCRIBE, email to debian-med-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
