Permalink
Browse files

BUG: cluster: 'linkage' C function in hierarchy.c did not check the r…

…eturn

value of malloc, which could lead to seg faults. This commit is a rebased
version of #110
Closes ticket #1562.
  • Loading branch information...
1 parent b5838b9 commit 7c03d21d84190f7ca4277b72caba5288d2928381 Warren Weckesser committed Nov 18, 2011
Showing with 52 additions and 17 deletions.
  1. +37 −11 scipy/cluster/src/hierarchy.c
  2. +1 −1 scipy/cluster/src/hierarchy.h
  3. +14 −5 scipy/cluster/src/hierarchy_wrap.c
@@ -69,6 +69,7 @@
#include "hierarchy.h"
+
static NPY_INLINE double euclidean_distance(const double *u, const double *v, int n) {
int i = 0;
double s = 0.0, d;
@@ -349,6 +350,7 @@ void print_vec(const double *d, int n) {
CPY_DEBUG_MSG("]");
}
+
/**
* notes to self:
* dm: The distance matrix.
@@ -358,42 +360,52 @@ void print_vec(const double *d, int n) {
* ml: A boolean indicating whether a list of objects in the forest
* clusters should be maintained.
* kc: Keep track of the centroids.
+ *
+ * Return values:
+ * 0: success
+ * -1: out of memory--malloc() failed.
*/
-void linkage(double *dm, double *Z, double *X,
+int linkage(double *dm, double *Z, double *X,
int m, int n, int ml, int kc, distfunc dfunc,
int method) {
int i, j, k, t, np, nid, mini, minj, npc2;
double min, ln, rn, qn;
- int *ind;
+ int *ind = NULL;
/** An iterator through the distance matrix. */
- double *dmit, *buf;
+ double *dmit, *buf = NULL;
- int *rowsize;
+ int *rowsize = NULL;
/** Temporary array to store modified distance matrix. */
- double *dmt, **rows, *Zrow;
- double *centroidsData;
- double **centroids;
+ double *dmt = NULL, **rows = NULL, *Zrow;
+ double *centroidsData = NULL;
+ double **centroids = NULL;
const double *centroidL, *centroidR;
double *centroid;
- clist *lists, *listL, *listR, *listC;
- clnode *lnodes;
- cnode *nodes, *node;
+ clist *lists = NULL, *listL, *listR, *listC;
+ clnode *lnodes = NULL;
+ cnode *nodes = NULL, *node;
cinfo info;
+ int result = -1;
+
/** The next two are only necessary for euclidean distance methods. */
if (ml) {
lists = (clist*)malloc(sizeof(clist) * (n-1));
+ if (!lists) goto finished;
lnodes = (clnode*)malloc(sizeof(clnode) * n);
+ if (!lnodes) goto finished;
}
else {
lists = 0;
lnodes = 0;
}
if (kc) {
centroids = (double**)malloc(sizeof(double*) * (2 * n));
+ if (!centroids) goto finished;
centroidsData = (double*)malloc(sizeof(double) * n * m);
+ if (!centroidsData) goto finished;
for (i = 0; i < n; i++) {
centroids[i] = X + i * m;
}
@@ -407,11 +419,17 @@ void linkage(double *dm, double *Z, double *X,
}
nodes = (cnode*)malloc(sizeof(cnode) * (n * 2) - 1);
+ if (!nodes) goto finished;
ind = (int*)malloc(sizeof(int) * n);
+ if (!ind) goto finished;
dmt = (double*)malloc(sizeof(double) * NCHOOSE2(n));
+ if (!dmt) goto finished;
buf = (double*)malloc(sizeof(double) * n);
+ if (!buf) goto finished;
rows = (double**)malloc(sizeof(double*) * n);
+ if (!rows) goto finished;
rowsize = (int*)malloc(sizeof(int) * n);
+ if (!rowsize) goto finished;
memcpy(dmt, dm, sizeof(double) * NCHOOSE2(n));
info.X = X;
@@ -600,6 +618,9 @@ void linkage(double *dm, double *Z, double *X,
ind[np - 2] = nid;
/** print_ind(ind, np - 1);**/
}
+ result = 0;
+
+finished:
free(lists);
free(lnodes);
free(nodes);
@@ -610,11 +631,16 @@ void linkage(double *dm, double *Z, double *X,
free(rowsize);
free(centroidsData);
free(centroids);
+ return result;
}
/** Trying to reimplement so that output is consistent with MATLAB's in
cases where there are is than one correct choice to make at each
- iteration of the algorithm. This implementation is not active. */
+ iteration of the algorithm. This implementation is not active.
+
+ XXX linkage_alt is not used. If it ever does get used, it needs to
+ be updated to check the results of any calls to malloc().
+*/
void linkage_alt(double *dm, double *Z, double *X,
int m, int n, int ml, int kc, distfunc dfunc,
@@ -102,7 +102,7 @@ void dist_weighted(cinfo *info, int mini, int minj, int np, int n);
int leaders(const double *Z, const int *T, int *L, int *M, int kk, int n);
-void linkage(double *dm, double *Z, double *X, int m, int n, int ml, int kc, distfunc dfunc, int method);
+int linkage(double *dm, double *Z, double *X, int m, int n, int ml, int kc, distfunc dfunc, int method);
void linkage_alt(double *dm, double *Z, double *X, int m, int n, int ml, int kc, distfunc dfunc, int method);
void cophenetic_distances(const double *Z, double *d, int n);
@@ -48,7 +48,7 @@ extern PyObject *linkage_wrap(PyObject *self, PyObject *args) {
&PyArray_Type, &Z,
&n,
&method)) {
- return 0;
+ return NULL;
}
else {
switch (method) {
@@ -69,7 +69,12 @@ extern PyObject *linkage_wrap(PyObject *self, PyObject *args) {
df = 0;
break;
}
- linkage((double*)dm->data, (double*)Z->data, 0, 0, n, 0, 0, df, method);
+ if (linkage((double*)dm->data, (double*)Z->data,
+ 0, 0, n, 0, 0, df, method) == -1) {
+ PyErr_SetString(PyExc_MemoryError,
+ "out of memory while computing linkage");
+ return NULL;
+ }
}
return Py_BuildValue("d", 0.0);
}
@@ -85,7 +90,7 @@ extern PyObject *linkage_euclid_wrap(PyObject *self, PyObject *args) {
&m,
&n,
&method)) {
- return 0;
+ return NULL;
}
else {
ml = 0;
@@ -106,8 +111,12 @@ extern PyObject *linkage_euclid_wrap(PyObject *self, PyObject *args) {
df = 0;
break;
}
- linkage((double*)dm->data, (double*)Z->data, (double*)X->data,
- m, n, 1, 1, df, method);
+ if (linkage((double*)dm->data, (double*)Z->data, (double*)X->data,
+ m, n, 1, 1, df, method) == -1) {
+ PyErr_SetString(PyExc_MemoryError,
+ "out of memory while computing linkage");
+ return NULL;
+ }
}
return Py_BuildValue("d", 0.0);
}

0 comments on commit 7c03d21

Please sign in to comment.