Cleanup lib{linear,svm} C helper routines

* Plumb memory leaks in allocation
* Don't cast return value from malloc
* Remove unused variables
* No more register keyword; is a no-op in modern compilers
* Cosmetic changes

Author: larsmans

scikits/learn/svm/src/liblinear/liblinear_helper.c

@@ -18,15 +18,18 @@
 struct feature_node **dense_to_sparse (double *x, npy_intp *dims, double bias)
 {
     struct feature_node **sparse;
-    register int i, j;              /* number of nonzero elements in row i */
+    int i, j;                           /* number of nonzero elements in row i */
     struct feature_node *temp;          /* stack for nonzero elements */
     struct feature_node *T;             /* pointer to the top of the stack */
     int count;
 
-    sparse = (struct feature_node **) malloc (dims[0] * sizeof(struct feature_node *));
-    temp = (struct feature_node *) malloc ((dims[1]+2) * sizeof(struct feature_node));
+    sparse = malloc (dims[0] * sizeof(struct feature_node *));
+    if (sparse == NULL)
+        goto sparse_error;
 
-    if (sparse == NULL || temp == NULL) return NULL;
+    temp = malloc ((dims[1]+2) * sizeof(struct feature_node));
+    if (temp == NULL)
+        goto temp_error;
 
     for (i=0; i<dims[0]; ++i) {
         T = temp; /* reset stack pointer */
@@ -53,13 +56,25 @@ struct feature_node **dense_to_sparse (double *x, npy_intp *dims, double bias)
 
         /* allocate memory and copy collected items*/
         count = T - temp;
-        sparse[i] = (struct feature_node *) malloc(count * sizeof(struct feature_node));
-        if (sparse[i] == NULL) return NULL;
+        sparse[i] = malloc(count * sizeof(struct feature_node));
+        if (sparse[i] == NULL) {
+            int k;
+            for (k=0; k<i; k++)
+                free(sparse[i]);
+            goto sparse_i_error;
+        }
         memcpy(sparse[i], temp, count * sizeof(struct feature_node));
     }
 
     free(temp);
     return sparse;
+
+sparse_i_error:
+    free(temp);
+temp_error:
+    free(sparse);
+sparse_error:
+    return NULL;
 }
 
 
@@ -72,11 +87,22 @@ struct feature_node **csr_to_sparse (double *values, npy_intp *shape_indices,
 {
     struct feature_node **sparse, *temp;
     int i, j=0, k=0, n;
-    sparse = (struct feature_node **) malloc ((shape_indptr[0]-1)* sizeof(struct feature_node *));
+
+    sparse = malloc ((shape_indptr[0]-1)* sizeof(struct feature_node *));
+    if (sparse == NULL)
+        return NULL;
 
     for (i=0; i<shape_indptr[0]-1; ++i) {
         n = indptr[i+1] - indptr[i]; /* count elements in row i */
-        sparse[i] = (struct feature_node *) malloc ((n+2) * sizeof(struct feature_node));
+
+        sparse[i] = malloc ((n+2) * sizeof(struct feature_node));
+        if (sparse[i] == NULL) {
+            int l;
+            for (l=0; l<i; l++)
+                free(sparse[i]);
+            break;
+        }
+
         temp = sparse[i];
         for (j=0; j<n; ++j) {
             temp[j].value = values[k];
@@ -101,7 +127,7 @@ struct problem * set_problem(char *X,char *Y, npy_intp *dims, double bias)
 {
     struct problem *problem;
     /* not performant but simple */
-    problem = (struct problem *) malloc(sizeof(struct problem));
+    problem = malloc(sizeof(struct problem));
     if (problem == NULL) return NULL;
     problem->l = (int) dims[0];
 
@@ -127,7 +153,7 @@ struct problem * csr_set_problem (char *values, npy_intp *n_indices,
         npy_intp n_features, double bias) {
 
     struct problem *problem;
-    problem = (struct problem *) malloc (sizeof (struct problem));
+    problem = malloc (sizeof (struct problem));
     if (problem == NULL) return NULL;
     problem->l = (int) n_indptr[0] -1;
 
@@ -155,7 +181,7 @@ struct problem * csr_set_problem (char *values, npy_intp *n_indices,
 struct parameter * set_parameter(int solver_type, double eps, double C, npy_intp nr_weight, char *weight_label, char *weight)
 {
     struct parameter *param;
-    param = (struct parameter *) malloc(sizeof(struct parameter));
+    param = malloc(sizeof(struct parameter));
     if (param == NULL) return NULL;
     param->solver_type = solver_type;
     param->eps = eps;
@@ -174,9 +200,12 @@ struct model * set_model(struct parameter *param, char *coef, npy_intp *dims,
     struct model *model;
 
     if (m == 1) m = 2; /* liblinear collapses the weight vector in the case of two classes */
-    model = (struct model *)      malloc(sizeof(struct model));
-    model->w =       (double *)   malloc( len_w * sizeof(double)); 
-    model->label =   (int *)      malloc( m * sizeof(int));
+    if ((model = malloc(sizeof(struct model))) == NULL)
+        goto model_error;
+    if ((model->w = malloc( len_w * sizeof(double))) == NULL)
+        goto w_error;
+    if ((model->label = malloc( m * sizeof(int))) == NULL)
+        goto label_error;
 
     memcpy(model->label, label, m * sizeof(int));
     memcpy(model->w, coef, len_w * sizeof(double));
@@ -188,6 +217,13 @@ struct model * set_model(struct parameter *param, char *coef, npy_intp *dims,
     model->bias = bias;
 
     return model;
+
+label_error:
+    free(model->w);
+w_error:
+    free(model);
+model_error:
+    return NULL;
 }
 
 
@@ -218,7 +254,7 @@ int copy_predict(char *train, struct model *model_, npy_intp *train_dims,
                  char *dec_values)
 {
     int *t = (int *) dec_values;
-    register int i, n;
+    int i, n;
     struct feature_node **train_nodes;
     n = train_dims[0];
     train_nodes = dense_to_sparse((double *) train, train_dims, model_->bias);
@@ -238,7 +274,8 @@ int copy_predict(char *train, struct model *model_, npy_intp *train_dims,
 int csr_copy_predict(npy_intp n_features, npy_intp *data_size, char *data,
         npy_intp *index_size,
         char *index, npy_intp *indptr_shape, char *intptr, struct model *model_,
-        char *dec_values) {
+        char *dec_values)
+{
     int *t = (int *) dec_values;
     struct feature_node **predict_nodes;
     npy_intp i;
@@ -279,9 +316,8 @@ int csr_copy_predict_values(npy_intp n_features, npy_intp *data_size,
                             char *data, npy_intp *index_size, char
                             *index, npy_intp *indptr_shape, char
                             *intptr, struct model *model_, char
-                            *dec_values, int nr_class) {
-
-    int *t = (int *) dec_values;
+                            *dec_values, int nr_class)
+{
     struct feature_node **predict_nodes;
     npy_intp i;
 
@@ -304,7 +340,7 @@ int copy_prob_predict(char *predict, struct model *model_, npy_intp *predict_dim
                  char *dec_values)
 {
     struct feature_node **predict_nodes;
-    register int i;
+    int i;
     int n, m;
     n = predict_dims[0];
     m = model_->nr_class;
@@ -328,8 +364,7 @@ int csr_copy_predict_proba(npy_intp n_features, npy_intp *data_size,
                            char *dec_values)
 {
     struct feature_node **predict_nodes;
-    register int i;
-    double temp;
+    int i;
     double *tx = (double *) dec_values;
 
     predict_nodes = csr_to_sparse((double *) data, index_size,

scikits/learn/svm/src/libsvm/libsvm_helper.c

@@ -37,7 +37,7 @@ struct svm_node *dense_to_libsvm (double *x, npy_intp *dims)
     double *tx = x;
     int i;
 
-    node = (struct svm_node *) malloc (dims[0] * sizeof(struct svm_node));
+    node = malloc (dims[0] * sizeof(struct svm_node));
 
     if (node == NULL) return NULL;
     for (i=0; i<dims[0]; ++i) {
@@ -53,7 +53,7 @@ struct svm_node *dense_to_libsvm (double *x, npy_intp *dims)
 
 
 /*
- * Create a svm_paramater struct and return it. It is up to the user to
+ * Create a svm_parameter struct and return it. It is up to the user to
  * free the resulting object.
  */
 struct svm_parameter * set_parameter(int svm_type, int kernel_type, int degree,
@@ -62,7 +62,7 @@ struct svm_parameter * set_parameter(int svm_type, int kernel_type, int degree,
 		char *weight_label, char *weight)
 {
     struct svm_parameter *param;
-    param = (struct svm_parameter *) malloc(sizeof(struct svm_parameter));
+    param = malloc(sizeof(struct svm_parameter));
     if (param == NULL) return NULL;
     param->svm_type = svm_type;
     param->kernel_type = kernel_type;
@@ -89,9 +89,8 @@ struct svm_parameter * set_parameter(int svm_type, int kernel_type, int degree,
 struct svm_problem * set_problem(char *X, char *Y, char *sample_weight, npy_intp *dims, int kernel_type)
 {
     struct svm_problem *problem;
-    int i;
 
-    problem = (struct svm_problem *) malloc(sizeof(struct svm_problem));
+    problem = malloc(sizeof(struct svm_problem));
     if (problem == NULL) return NULL;
     problem->l = (int) dims[0]; /* number of samples */
     problem->y = (double *) Y;
@@ -131,18 +130,24 @@ struct svm_model *set_model(struct svm_parameter *param, int nr_class,
 
     m = nr_class * (nr_class-1)/2;
 
-    model = (struct svm_model *)  malloc(sizeof(struct svm_model));
-    model->nSV =     (int *)      malloc(nr_class * sizeof(int));
-    model->label =   (int *)      malloc(nr_class * sizeof(int));;
-    model->sv_coef = (double **)  malloc((nr_class-1)*sizeof(double *));
-    model->rho =     (double *)   malloc( m * sizeof(double));
+    if ((model = malloc(sizeof(struct svm_model))) == NULL)
+        goto model_error;
+    if ((model->nSV = malloc(nr_class * sizeof(int))) == NULL)
+        goto nsv_error;
+    if ((model->label = malloc(nr_class * sizeof(int))) == NULL)
+        goto label_error;
+    if ((model->sv_coef = malloc((nr_class-1)*sizeof(double *))) == NULL)
+        goto sv_coef_error;
+    if ((model->rho = malloc( m * sizeof(double))) == NULL)
+        goto rho_error;
 
     model->nr_class = nr_class;
     model->param = *param;
     model->l = (int) support_dims[0];
 
     if (param->kernel_type == PRECOMPUTED) {
-        model->SV = (struct svm_node *) malloc ((model->l) * sizeof(struct svm_node));
+        if ((model->SV = malloc ((model->l) * sizeof(struct svm_node))) == NULL)
+            goto SV_error;
         for (i=0; i<model->l; ++i) {
             model->SV[i].ind = ((int *) support)[i];
             model->SV[i].values = NULL;
@@ -173,9 +178,11 @@ struct svm_model *set_model(struct svm_parameter *param, int nr_class,
      */
 
     if (param->probability) {
-        model->probA = (double *) malloc(m * sizeof(double));
+        if ((model->probA = malloc(m * sizeof(double))) == NULL)
+            goto probA_error;
         memcpy(model->probA, probA, m * sizeof(double));
-        model->probB = (double *) malloc(m * sizeof(double));
+        if ((model->probB = malloc(m * sizeof(double))) == NULL)
+            goto probB_error;
         memcpy(model->probB, probB, m * sizeof(double));
     } else {
         model->probA = NULL;
@@ -185,6 +192,23 @@ struct svm_model *set_model(struct svm_parameter *param, int nr_class,
     /* We'll free SV ourselves */
     model->free_sv = 0;
     return model;
+
+probB_error:
+    free(model->probA);
+probA_error:
+    free(model->SV);
+SV_error:
+    free(model->rho);
+rho_error:
+    free(model->sv_coef);
+sv_coef_error:
+    free(model->label);
+label_error:
+    free(model->nSV);
+nsv_error:
+    free(model);
+model_error:
+    return NULL;
 }
 
 
@@ -241,7 +265,7 @@ void copy_intercept(char *data, struct svm_model *model, npy_intp *dims)
  */
 void copy_SV(char *data, struct svm_model *model, npy_intp *dims)
 {
-    int i, j, k, n = model->l;
+    int i, n = model->l;
     double *tdata = (double *) data;
     int dim = model->SV[0].dim;
     for (i=0; i<n; ++i) {
@@ -250,9 +274,9 @@ void copy_SV(char *data, struct svm_model *model, npy_intp *dims)
     }
 }
 
-int copy_support (char *data, struct svm_model *model)
+void copy_support (char *data, struct svm_model *model)
 {
-        memcpy (data, model->sv_ind, (model->l) * sizeof(int));
+    memcpy (data, model->sv_ind, (model->l) * sizeof(int));
 }
 
 /* 
@@ -354,7 +378,6 @@ int copy_predict_proba(char *predict, struct svm_model *model, npy_intp *predict
  */
 int free_problem(struct svm_problem *problem)
 {
-    register int i;
     if (problem == NULL) return -1;
     free (problem->x);
     free (problem);