Merge pull request #6951 from badasahog/devBranch1

changed malloc to use objectwise sizeof and exclude explicit pointer type cast

Author: MichaelChirico

.ci/linters/cocci/malloc_return_value_cast.cocci

@@ -0,0 +1,6 @@
+@malloc_return_value_cast expression@
+type T;
+expression E;
+@@
+- (T)
+  malloc(E)

src/assign.c

@@ -646,7 +646,7 @@ SEXP assign(SEXP dt, SEXP rows, SEXP cols, SEXP newcolnames, SEXP values)
       if (!*tc1) internal_error(__func__, "index name ends with trailing __"); // # nocov
       // check the position of the first appearance of an assigned column in the index.
       // the new index will be truncated to this position.
-      char *s4 = (char*) malloc(strlen(c1) + 3);
+      char *s4 = malloc(strlen(c1) + 3);
       if (!s4) {
         internal_error(__func__, "Couldn't allocate memory for s4"); // # nocov
       }
@@ -656,7 +656,7 @@ SEXP assign(SEXP dt, SEXP rows, SEXP cols, SEXP newcolnames, SEXP values)
       int newKeyLength = strlen(c1);
       for(int i = 0; i < xlength(assignedNames); i++){
         tc2 = CHAR(STRING_ELT(assignedNames, i));
-        char *s5 = (char*) malloc(strlen(tc2) + 5); //4 * '_' + \0
+        char *s5 = malloc(strlen(tc2) + 5); //4 * '_' + \0
         if (!s5) {
           free(s4);                                                  // # nocov
           internal_error(__func__, "Couldn't allocate memory for s5"); // # nocov
@@ -873,7 +873,7 @@ const char *memrecycle(const SEXP target, const SEXP where, const int start, con
         for (int k=0; k<nTargetLevels; ++k) SET_TRUELENGTH(targetLevelsD[k], 0);  // don't need those anymore
         if (nAdd) {
           // cannot grow the levels yet as that would be R call which could fail to alloc and we have no hook to clear up
-          SEXP *temp = (SEXP *)malloc(nAdd * sizeof(SEXP *));
+          SEXP *temp = malloc(sizeof(*temp) * nAdd);
           if (!temp) {
             // # nocov start
             for (int k=0; k<nSourceLevels; ++k) SET_TRUELENGTH(sourceLevelsD[k], 0);
@@ -1282,8 +1282,8 @@ void savetl_init(void) {
   }
   nsaved = 0;
   nalloc = 100;
-  saveds = (SEXP *)malloc(nalloc * sizeof(SEXP));
-  savedtl = (R_len_t *)malloc(nalloc * sizeof(R_len_t));
+  saveds = malloc(sizeof(*saveds) * nalloc);
+  savedtl = malloc(sizeof(*savedtl) * nalloc);
   if (!saveds || !savedtl) {
     free(saveds); free(savedtl);                                            // # nocov
     savetl_end();                                                           // # nocov

src/forder.c

@@ -280,8 +280,8 @@ static void cradix_r(SEXP *xsub, int n, int radix)
 
 static void cradix(SEXP *x, int n)
 {
-  cradix_counts = (int *)calloc(ustr_maxlen*256, sizeof(int));  // counts for the letters of left-aligned strings
-  cradix_xtmp = (SEXP *)malloc(ustr_n*sizeof(SEXP));
+  cradix_counts = calloc(ustr_maxlen*256, sizeof(*cradix_counts));  // counts for the letters of left-aligned strings
+  cradix_xtmp = malloc(sizeof(*cradix_xtmp) * ustr_n);
   if (!cradix_counts || !cradix_xtmp) {
     free(cradix_counts); free(cradix_xtmp); // # nocov
     STOP(_("Failed to alloc cradix_counts and/or cradix_tmp")); // # nocov
@@ -342,7 +342,7 @@ static void range_str(const SEXP *x, int n, uint64_t *out_min, uint64_t *out_max
   if (anynotutf8) {
     SEXP ustr2 = PROTECT(allocVector(STRSXP, ustr_n));
     for (int i=0; i<ustr_n; i++) SET_STRING_ELT(ustr2, i, ENC2UTF8(ustr[i]));
-    SEXP *ustr3 = (SEXP *)malloc(ustr_n * sizeof(SEXP));
+    SEXP *ustr3 = malloc(sizeof(*ustr3) * ustr_n);
     if (!ustr3)
       STOP(_("Failed to alloc ustr3 when converting strings to UTF8"));  // # nocov
     memcpy(ustr3, STRING_PTR_RO(ustr2), ustr_n*sizeof(SEXP));
@@ -361,7 +361,7 @@ static void range_str(const SEXP *x, int n, uint64_t *out_min, uint64_t *out_max
       SET_TRUELENGTH(ustr3[i], --o);
     }
     // now use the 1-1 mapping from ustr to ustr2 to get the ordering back into original ustr, being careful to reset tl to 0
-    int *tl = (int *)malloc(ustr_n * sizeof(int));
+    int *tl = malloc(sizeof(*tl) * ustr_n);
     if (!tl) {
       free(ustr3); // # nocov
       STOP(_("Failed to alloc tl when converting strings to UTF8"));  // # nocov
@@ -788,8 +788,8 @@ SEXP forder(SEXP DT, SEXP by, SEXP retGrpArg, SEXP retStatsArg, SEXP sortGroupsA
 
   // global nth, TMP & UGRP
   nth = getDTthreads(nrow, true);  // this nth is relied on in cleanup(); throttle=true/false debated for #5077
-  TMP =  (int *)malloc(nth*UINT16_MAX*sizeof(int)); // used by counting sort (my_n<=65536) in radix_r()
-  UGRP = (uint8_t *)malloc(nth*256);                // TODO: align TMP and UGRP to cache lines (and do the same for stack allocations too)
+  TMP =  malloc(sizeof(*TMP)*nth*UINT16_MAX); // used by counting sort (my_n<=65536) in radix_r()
+  UGRP = malloc(sizeof(*UGRP)*nth*256);                // TODO: align TMP and UGRP to cache lines (and do the same for stack allocations too)
   if (!TMP || !UGRP /*|| TMP%64 || UGRP%64*/) {
     free(TMP); free(UGRP); // # nocov
     STOP(_("Failed to allocate TMP or UGRP or they weren't cache line aligned: nth=%d"), nth); // # nocov
@@ -917,7 +917,7 @@ void radix_r(const int from, const int to, const int radix) {
     #endif
 
     uint8_t *restrict my_key = key[radix]+from;  // safe to write as we don't use this radix again
-    uint8_t *o = (uint8_t *)malloc(my_n * sizeof(uint8_t));
+    uint8_t *o = malloc(sizeof(*o) * my_n);
     if (!o)
       STOP(_("Failed to allocate %d bytes for '%s'."), (int)(my_n * sizeof(uint8_t)), "o"); // # nocov
     // if last key (i.e. radix+1==nradix) there are no more keys to reorder so we could reorder osub by reference directly and save allocating and populating o just
@@ -986,10 +986,10 @@ void radix_r(const int from, const int to, const int radix) {
     }
     if (!skip) {
       // reorder osub and each remaining ksub
-      int *TMP = malloc(my_n * sizeof(int));
+      int *TMP = malloc(sizeof(*TMP) * my_n);
       if (!TMP) {
         free(o); // # nocov
-        STOP(_("Failed to allocate %d bytes for '%s'."), (int)(my_n * sizeof(int)), "TMP"); // # nocov
+        STOP(_("Failed to allocate %d bytes for '%s'."), (int)(sizeof(*TMP) * my_n), "TMP"); // # nocov
       }
       const int *restrict osub = anso+from;
       for (int i=0; i<my_n; i++) TMP[i] = osub[o[i]];
@@ -1009,9 +1009,9 @@ void radix_r(const int from, const int to, const int radix) {
       return;
     }
     int ngrp=0; //minor TODO: could know number of groups with certainty up above
-    int *my_gs = malloc(my_n * sizeof(int));
+    int *my_gs = malloc(sizeof(*my_gs) * my_n);
     if (!my_gs)
-      STOP(_("Failed to allocate %d bytes for '%s'."), (int)(my_n * sizeof(int)), "my_gs"); // # nocov
+      STOP(_("Failed to allocate %d bytes for '%s'."), (int)(sizeof(*my_gs) * my_n), "my_gs"); // # nocov
     my_gs[ngrp]=1;
     for (int i=1; i<my_n; i++) {
       if (my_key[i]!=my_key[i-1]) my_gs[++ngrp] = 1;
@@ -1111,7 +1111,7 @@ void radix_r(const int from, const int to, const int radix) {
     if (!retgrp && radix+1==nradix) {
       return;  // we're done. avoid allocating and populating very last group sizes for last key
     }
-    int *my_gs = malloc((ngrp==0 ? 256 : ngrp) * sizeof(int)); // ngrp==0 when sort and skip==true; we didn't count the non-zeros in my_counts yet in that case
+    int *my_gs = malloc(sizeof(*my_gs) * (ngrp==0 ? 256 : ngrp)); // ngrp==0 when sort and skip==true; we didn't count the non-zeros in my_counts yet in that case
     if (!my_gs)
       STOP(_("Failed to allocate %d bytes for '%s'."), (int)((ngrp==0 ? 256 : ngrp) * sizeof(int)), "my_gs"); // # nocov
     if (sortType!=0) {
@@ -1139,9 +1139,9 @@ void radix_r(const int from, const int to, const int radix) {
   int batchSize = MIN(UINT16_MAX, 1+my_n/getDTthreads(my_n, true));  // (my_n-1)/nBatch + 1;   //UINT16_MAX == 65535
   int nBatch = (my_n-1)/batchSize + 1;   // TODO: make nBatch a multiple of nThreads?
   int lastBatchSize = my_n - (nBatch-1)*batchSize;
-  uint16_t *counts = calloc(nBatch*256,sizeof(uint16_t));
-  uint8_t  *ugrps =  malloc(nBatch*256*sizeof(uint8_t));
-  int      *ngrps =  calloc(nBatch    ,sizeof(int));
+  uint16_t *counts = calloc(nBatch*256,sizeof(*counts));
+  uint8_t  *ugrps =  malloc(sizeof(*ugrps)*nBatch*256);
+  int      *ngrps =  calloc(nBatch    ,sizeof(*ngrps));
   if (!counts || !ugrps || !ngrps) {
     free(counts); free(ugrps); free(ngrps); // # nocov
     STOP(_("Failed to allocate parallel counts. my_n=%d, nBatch=%d"), my_n, nBatch); // # nocov
@@ -1152,11 +1152,11 @@ void radix_r(const int from, const int to, const int radix) {
   TEND(16)
   #pragma omp parallel num_threads(getDTthreads(nBatch, false))
   {
-    int     *my_otmp = malloc(batchSize * sizeof(int)); // thread-private write
-    uint8_t *my_ktmp = malloc(batchSize * sizeof(uint8_t) * n_rem);
+    int     *my_otmp = malloc(sizeof(*my_otmp) * batchSize); // thread-private write
+    uint8_t *my_ktmp = malloc(sizeof(*my_ktmp) * batchSize * n_rem);
     if (!my_otmp || !my_ktmp) {
       free(my_otmp); free(my_ktmp);
-      STOP(_("Failed to allocate 'my_otmp' and/or 'my_ktmp' arrays (%d bytes)."), (int)(batchSize*(sizeof(int) + sizeof(uint8_t))));
+      STOP(_("Failed to allocate 'my_otmp' and/or 'my_ktmp' arrays (%d bytes)."), (int)((sizeof(*my_otmp) + sizeof(*my_ktmp)) * batchSize));
     }
     // TODO: move these up above and point restrict[me] to them. Easier to Error that way if failed to alloc.
     #pragma omp for
@@ -1259,7 +1259,7 @@ void radix_r(const int from, const int to, const int radix) {
 
   TEND(18 + notFirst*3)
   if (!skip) {
-    int *TMP = malloc(my_n * sizeof(int));
+    int *TMP = malloc(sizeof(*TMP) * my_n);
     if (!TMP)
       STOP(_("Unable to allocate TMP for my_n=%d items in parallel batch counting"), my_n); // # nocov
     #pragma omp parallel for num_threads(getDTthreads(nBatch, false))
@@ -1298,7 +1298,7 @@ void radix_r(const int from, const int to, const int radix) {
   TEND(19 + notFirst*3)
   notFirst = true;
 
-  int *my_gs = malloc(ngrp * sizeof(int));
+  int *my_gs = malloc(sizeof(*my_gs) * ngrp);
   if (!my_gs)
     STOP(_("Failed to allocate %d bytes for '%s'."), (int)(ngrp * sizeof(int)), "my_gs"); // # nocov
   for (int i=1; i<ngrp; i++) my_gs[i-1] = starts[ugrp[i]] - starts[ugrp[i-1]];   // use the first row of starts to get totals

src/fread.c

@@ -441,7 +441,7 @@ static const char* filesize_to_str(size_t fsize)
 double copyFile(size_t fileSize)  // only called in very very rare cases
 {
   double tt = wallclock();
-  mmp_copy = (char *)malloc(fileSize + 1 /* extra \0 */);
+  mmp_copy = malloc(fileSize + 1 /* extra \0 */);
   if (!mmp_copy)
     return -1.0; // # nocov
   memcpy(mmp_copy, mmp, fileSize);
@@ -1884,8 +1884,8 @@ int freadMain(freadMainArgs _args) {
   if (verbose) DTPRINT(_("[07] Detect column types, dec, good nrow estimate and whether first row is column names\n"));
   if (verbose && args.header!=NA_BOOL8) DTPRINT(_("  'header' changed by user from 'auto' to %s\n"), args.header?"true":"false");
 
-  type =    (int8_t *)malloc((size_t)ncol * sizeof(int8_t));
-  tmpType = (int8_t *)malloc((size_t)ncol * sizeof(int8_t));  // used i) in sampling to not stop on errors when bad jump point and ii) when accepting user overrides
+  type =    malloc(sizeof(*type) * (size_t)ncol);
+  tmpType = malloc(sizeof(*tmpType) * (size_t)ncol);  // used i) in sampling to not stop on errors when bad jump point and ii) when accepting user overrides
   if (!type || !tmpType) {
     free(type); free(tmpType); // # nocov
     STOP(_("Failed to allocate 2 x %d bytes for type and tmpType: %s"), ncol, strerror(errno)); // # nocov
@@ -2201,9 +2201,9 @@ int freadMain(freadMainArgs _args) {
   rowSize1 = 0;
   rowSize4 = 0;
   rowSize8 = 0;
-  size = (int8_t *)malloc((size_t)ncol * sizeof(int8_t));  // TODO: remove size[] when we implement Pasha's idea to += size inside processor
+  size = malloc(sizeof(*size) * (size_t)ncol);  // TODO: remove size[] when we implement Pasha's idea to += size inside processor
   if (!size)
-    STOP(_("Failed to allocate %d bytes for '%s': %s"), (int)(ncol * sizeof(int8_t)), "size", strerror(errno)); // # nocov
+    STOP(_("Failed to allocate %d bytes for '%s': %s"), (int)(sizeof(*size) * (size_t)ncol), "size", strerror(errno)); // # nocov
   nStringCols = 0;
   nNonStringCols = 0;
   for (int j=0; j<ncol; j++) {
@@ -2642,7 +2642,7 @@ int freadMain(freadMainArgs _args) {
       DTPRINT(_("  Provided number of fill columns: %d but only found %d\n"), ncol, max_col);
       DTPRINT(_("  Dropping %d overallocated columns\n"), ndropFill);
     }
-    dropFill = (int *)malloc((size_t)ndropFill * sizeof(int));
+    dropFill = malloc(sizeof(*dropFill) * (size_t)ndropFill);
     if (!dropFill)
       STOP(_("Failed to allocate %d bytes for '%s'."), (int)(ndropFill * sizeof(int)), "dropFill"); // # nocov
     int i=0;

src/frolladaptive.c

@@ -29,7 +29,7 @@ void fadaptiverollmeanFast(double *x, uint64_t nx, ans_t *ans, int *k, double fi
     snprintf(end(ans->message[0]), 500, _("%s: running for input length %"PRIu64", hasna %d, narm %d\n"), "fadaptiverollmeanFast", (uint64_t)nx, hasna, (int) narm);
   bool truehasna = hasna>0;                                     // flag to re-run if NAs detected
   long double w = 0.0;
-  double *cs = malloc(nx*sizeof(double));                       // cumsum vector, same as double cs[nx] but no segfault
+  double *cs = malloc(sizeof(*cs) * nx);                       // cumsum vector, same as double cs[nx] but no segfault
   if (!cs) {                                                    // # nocov start
     ans->status = 3;                                            // raise error
     snprintf(ans->message[3], 500, _("%s: Unable to allocate memory for cumsum"), __func__);
@@ -65,7 +65,7 @@ void fadaptiverollmeanFast(double *x, uint64_t nx, ans_t *ans, int *k, double fi
   }
   if (truehasna) {
     uint64_t nc = 0;                                            // running NA counter
-    uint64_t *cn = malloc(nx*sizeof(uint64_t));                 // cumulative NA counter, used the same way as cumsum, same as uint64_t cn[nx] but no segfault
+    uint64_t *cn = malloc(sizeof(*cn) * nx);                 // cumulative NA counter, used the same way as cumsum, same as uint64_t cn[nx] but no segfault
     if (!cn) {                                                  // # nocov start
       ans->status = 3;                                          // raise error
       snprintf(ans->message[3], 500, _("%s: Unable to allocate memory for cum NA counter"), __func__);
@@ -218,7 +218,7 @@ void fadaptiverollsumFast(double *x, uint64_t nx, ans_t *ans, int *k, double fil
     snprintf(end(ans->message[0]), 500, _("%s: running for input length %"PRIu64", hasna %d, narm %d\n"), "fadaptiverollsumFast", (uint64_t)nx, hasna, (int) narm);
   bool truehasna = hasna>0;
   long double w = 0.0;
-  double *cs = malloc(nx*sizeof(double));
+  double *cs = malloc(sizeof(*cs) * nx);
   if (!cs) {                                                    // # nocov start
     ans->status = 3;
     snprintf(ans->message[3], 500, _("%s: Unable to allocate memory for cumsum"), __func__);
@@ -254,7 +254,7 @@ void fadaptiverollsumFast(double *x, uint64_t nx, ans_t *ans, int *k, double fil
   }
   if (truehasna) {
     uint64_t nc = 0;
-    uint64_t *cn = malloc(nx*sizeof(uint64_t));
+    uint64_t *cn = malloc(sizeof(*cn) * nx);
     if (!cn) {                                                  // # nocov start
       ans->status = 3;
       snprintf(ans->message[3], 500, _("%s: Unable to allocate memory for cum NA counter"), __func__);

src/fsort.c

@@ -140,8 +140,8 @@ SEXP fsort(SEXP x, SEXP verboseArg) {
   // and ii) for small vectors with just one batch
 
   t[1] = wallclock();
-  double *mins = (double *)malloc(nBatch * sizeof(double));
-  double *maxs = (double *)malloc(nBatch * sizeof(double));
+  double *mins = malloc(sizeof(*mins) * nBatch);
+  double *maxs = malloc(sizeof(*maxs) * nBatch);
   if (!mins || !maxs) {
     free(mins); free(maxs); // # nocov
     error(_("Failed to allocate %d bytes in fsort()."), (int)(2 * nBatch * sizeof(double))); // # nocov
@@ -300,7 +300,7 @@ SEXP fsort(SEXP x, SEXP verboseArg) {
         uint64_t thisN = msbCounts[order[msb]];
 
         if (myworking==NULL) {
-          myworking = malloc(thisN * sizeof(double));
+          myworking = malloc(sizeof(*myworking) * thisN);
           if (!myworking) {
             failed=true; alloc_fail=true; continue;  // # nocov
           }

src/gsumm.c

@@ -118,7 +118,7 @@ SEXP gforce(SEXP env, SEXP jsub, SEXP o, SEXP f, SEXP l, SEXP irowsArg) {
     int highSize = ((nrow-1)>>bitshift) + 1;
     //Rprintf(_("When assigning grp[o] = g, highSize=%d  nb=%d  bitshift=%d  nBatch=%d\n"), highSize, nb, bitshift, nBatch);
     int *counts = calloc(nBatch*highSize, sizeof(int));  // TODO: cache-line align and make highSize a multiple of 64
-    int *TMP   = malloc(nrow*2l*sizeof(int)); // must multiple the long int otherwise overflow may happen, #4295
+    int *TMP   = malloc(sizeof(*TMP) * nrow*2l); // must multiple the long int otherwise overflow may happen, #4295
     if (!counts || !TMP ) {
       free(counts); free(TMP); // # nocov
       error(_("Failed to allocate counts or TMP when assigning g in gforce")); // # nocov
@@ -1124,7 +1124,7 @@ SEXP gprod(SEXP x, SEXP narmArg) {
   const int n = nosubset ? length(x) : irowslen;
   //clock_t start = clock();
   if (nrow != n) error(_("nrow [%d] != length(x) [%d] in %s"), nrow, n, "gprod");
-  long double *s = malloc(ngrp * sizeof(long double));
+  long double *s = malloc(sizeof(*s) * ngrp);
   if (!s)
     error(_("Unable to allocate %d * %zu bytes for gprod"), ngrp, sizeof(long double)); // # nocov
   for (int i=0; i<ngrp; ++i) s[i] = 1.0;