improved consistency and clarity around allocation functions (#7029)

* more thorough analysis

* linter update

* terminal newline

* restore terminal newline

* restore terminal newline

* restore terminal newlines

* restore terminal newline

---------

Co-authored-by: Michael Chirico <[email protected]>

Author: badasahog

.ci/linters/cocci/malloc_return_value_cast.cocci

@@ -4,3 +4,9 @@ expression E;
 @@
 - (T)
   malloc(E)
+
+- (T)
+  calloc(_, E)
+
+- (T)
+  realloc(_, E)

src/assign.c

@@ -400,7 +400,7 @@ SEXP assign(SEXP dt, SEXP rows, SEXP cols, SEXP newcolnames, SEXP values)
   // FR #2077 - set able to add new cols by reference
   if (isString(cols)) {
     PROTECT(tmp = chmatch(cols, names, 0)); protecti++;
-    buf = (int *) R_alloc(length(cols), sizeof(int));
+    buf = (int *) R_alloc(length(cols), sizeof(*buf));
     int k=0;
     for (int i=0; i<length(cols); ++i) {
       if (INTEGER(tmp)[i] == 0) buf[k++] = i;
@@ -699,7 +699,7 @@ SEXP assign(SEXP dt, SEXP rows, SEXP cols, SEXP newcolnames, SEXP values)
   }
   if (ndelete) {
     // delete any columns assigned NULL (there was a 'continue' earlier in loop above)
-    int *tt = (int *)R_alloc(ndelete, sizeof(int));
+    int *tt = (int *)R_alloc(ndelete, sizeof(*tt));
     const int *colsd=INTEGER(cols), ncols=length(cols), ndt=length(dt);
     for (int i=0, k=0; i<ncols; ++i) {   // find which ones to delete and put them in tt
       // Aside: a new column being assigned NULL (something odd to do) would have been warned above, added above, and now deleted. Just
@@ -1055,7 +1055,7 @@ const char *memrecycle(const SEXP target, const SEXP where, const int start, con
     case RAWSXP:    BODY(Rbyte, RAW,    int, val!=0,                                    td[i]=cval)
     case LGLSXP:
       if (mc) {
-                    memcpy(td, LOGICAL_RO(source), slen*sizeof(int)); break;
+                    memcpy(td, LOGICAL_RO(source), slen*sizeof(*td)); break;
       } else        BODY(int, LOGICAL,  int, val,                                       td[i]=cval)
     case INTSXP:    BODY(int, INTEGER,  int, val==NA_INTEGER ? NA_LOGICAL : val!=0,     td[i]=cval)
     case REALSXP:
@@ -1072,7 +1072,7 @@ const char *memrecycle(const SEXP target, const SEXP where, const int start, con
     case  LGLSXP:   // same as INTSXP ...
     case  INTSXP:
       if (mc) {
-                    memcpy(td, INTEGER_RO(source), slen*sizeof(int)); break;
+                    memcpy(td, INTEGER_RO(source), slen*sizeof(*td)); break;
       } else        BODY(int, INTEGER,  int, val,                                       td[i]=cval)
     case REALSXP:
       if (sourceIsI64)
@@ -1092,7 +1092,7 @@ const char *memrecycle(const SEXP target, const SEXP where, const int start, con
       case REALSXP:
         if (sourceIsI64) {
           if (mc) {
-                    memcpy(td, (const int64_t *)REAL_RO(source), slen*sizeof(int64_t)); break;
+                    memcpy(td, (const int64_t *)REAL_RO(source), slen*sizeof(*td)); break;
           } else    BODY(int64_t, REAL, int64_t, val,                                   td[i]=cval)
         } else      BODY(double, REAL,  int64_t, within_int64_repres(val) ? val : NA_INTEGER64,    td[i]=cval)
       case CPLXSXP: BODY(Rcomplex, COMPLEX, int64_t, ISNAN(val.r) ? NA_INTEGER64 : (int64_t)val.r, td[i]=cval)
@@ -1291,14 +1291,14 @@ void savetl(SEXP s)
       internal_error(__func__, "reached maximum %d items for savetl", nalloc); // # nocov
     }
     nalloc = nalloc>(INT_MAX/2) ? INT_MAX : nalloc*2;
-    char *tmp = (char *)realloc(saveds, nalloc*sizeof(SEXP));
+    char *tmp = realloc(saveds, sizeof(SEXP)*nalloc);
     if (tmp==NULL) {
       // C spec states that if realloc() fails the original block is left untouched; it is not freed or moved. We rely on that here.
       savetl_end();                                                      // # nocov  free(saveds) happens inside savetl_end
       error(_("Failed to realloc saveds to %d items in savetl"), nalloc);   // # nocov
     }
     saveds = (SEXP *)tmp;
-    tmp = (char *)realloc(savedtl, nalloc*sizeof(R_len_t));
+    tmp = realloc(savedtl, sizeof(R_len_t)*nalloc);
     if (tmp==NULL) {
       savetl_end();                                                      // # nocov
       error(_("Failed to realloc savedtl to %d items in savetl"), nalloc);  // # nocov

src/chmatch.c

@@ -96,8 +96,8 @@ static SEXP chmatchMain(SEXP x, SEXP table, int nomatch, bool chin, bool chmatch
     // For example: A,B,C,B,D,E,A,A   =>   A(TL=1),B(2),C(3),D(4),E(5)   =>   dupMap    1  2  3  5  6 | 8  7  4
     //                                                                        dupLink   7  8          |    6     (blank=0)
     unsigned int mapsize = tablelen+nuniq; // lto compilation warning #5760 // +nuniq to store a 0 at the end of each group
-    int *counts = (int *)calloc(nuniq, sizeof(int));
-    int *map =    (int *)calloc(mapsize, sizeof(int));
+    int *counts = calloc(nuniq, sizeof(*counts));
+    int *map =    calloc(mapsize, sizeof(*map));
     if (!counts || !map) {
       // # nocov start
       free(counts); free(map);

src/cj.c

@@ -36,7 +36,7 @@ SEXP cj(SEXP base_list) {
       }
       #pragma omp parallel for num_threads(getDTthreads(ncopy*blocklen, true))
       for (int i=1; i<ncopy; ++i) {
-        memcpy(targetP + i*blocklen, targetP, blocklen*sizeof(int));
+        memcpy(targetP + i*blocklen, targetP, blocklen*sizeof(*targetP));
       }
     } break;
     case REALSXP: {

src/coalesce.c

@@ -53,7 +53,7 @@ SEXP coalesce(SEXP x, SEXP inplaceArg) {
     first = PROTECT(copyAsPlain(first)); nprotect++;
     if (verbose) Rprintf(_("coalesce copied first item (inplace=FALSE)\n"));
   }
-  const void **valP = (const void **)R_alloc(nval, sizeof(void *));
+  const void **valP = (const void **)R_alloc(nval, sizeof(*valP));
   switch(TYPEOF(first)) {
   case LGLSXP:
   case INTSXP: {

src/dogroups.c

@@ -139,7 +139,7 @@ SEXP dogroups(SEXP dt, SEXP dtcols, SEXP groups, SEXP grpcols, SEXP jiscols, SEX
   SEXP names = PROTECT(getAttrib(SDall, R_NamesSymbol)); nprotect++;
   if (length(names) != length(SDall))
     internal_error(__func__, "length(names)!=length(SD)"); // # nocov
-  SEXP *nameSyms = (SEXP *)R_alloc(length(names), sizeof(SEXP));
+  SEXP *nameSyms = (SEXP *)R_alloc(length(names), sizeof(*nameSyms));
 
   for(int i=0; i<length(SDall); ++i) {
     SEXP this = VECTOR_ELT(SDall, i);
@@ -156,7 +156,7 @@ SEXP dogroups(SEXP dt, SEXP dtcols, SEXP groups, SEXP grpcols, SEXP jiscols, SEX
   SEXP xknames = PROTECT(getAttrib(xSD, R_NamesSymbol)); nprotect++;
   if (length(xknames) != length(xSD))
     internal_error(__func__, "length(xknames)!=length(xSD)"); // # nocov
-  SEXP *xknameSyms = (SEXP *)R_alloc(length(xknames), sizeof(SEXP));
+  SEXP *xknameSyms = (SEXP *)R_alloc(length(xknames), sizeof(*xknameSyms));
   for(int i=0; i<length(xSD); ++i) {
     if (SIZEOF(VECTOR_ELT(xSD, i))==0)
       internal_error(__func__, "type %d in .xSD column %d should have been caught by now", TYPEOF(VECTOR_ELT(xSD, i)), i); // # nocov

src/fmelt.c

@@ -20,7 +20,7 @@ SEXP set_diff(SEXP x, int n) {
   SEXP table = PROTECT(seq_int(n, 1));       // TODO: using match to 1:n seems odd here, why use match at all
   SEXP xmatch = PROTECT(match(x, table, 0)); // Old comment:took a while to realise: matches vec against x - thanks to comment from Matt in assign.c!
   const int *ixmatch = INTEGER(xmatch);
-  int *buf = (int *) R_alloc(n, sizeof(int));
+  int *buf = (int *) R_alloc(n, sizeof(*buf));
   int j=0;
   for (int i=0; i<n; ++i) {
     if (ixmatch[i] == 0) {
@@ -40,7 +40,7 @@ SEXP which(SEXP x, Rboolean val) {
   SEXP ans;
   if (!isLogical(x)) error(_("Argument to 'which' must be logical"));
   const int *ix = LOGICAL(x);
-  int *buf = (int *) R_alloc(n, sizeof(int));
+  int *buf = (int *) R_alloc(n, sizeof(*buf));
   for (int i=0; i<n; ++i) {
     if (ix[i] == val) {
       buf[j++] = i+1;
@@ -317,10 +317,10 @@ static void preprocess(SEXP DT, SEXP id, SEXP measure, SEXP varnames, SEXP valna
   }
   if (length(varnames) != 1)
     error(_("'variable.name' must be a character/integer vector of length 1."));
-  data->leach = (int *)R_alloc(data->lvalues, sizeof(int));
-  data->isidentical = (int *)R_alloc(data->lvalues, sizeof(int));
-  data->isfactor = (int *)R_alloc(data->lvalues, sizeof(int));
-  data->maxtype = (SEXPTYPE *)R_alloc(data->lvalues, sizeof(SEXPTYPE));
+  data->leach = (int *)R_alloc(data->lvalues, sizeof(*data->leach));
+  data->isidentical = (int *)R_alloc(data->lvalues, sizeof(*data->isidentical));
+  data->isfactor = (int *)R_alloc(data->lvalues, sizeof(*data->isfactor));
+  data->maxtype = (SEXPTYPE *)R_alloc(data->lvalues, sizeof(*data->maxtype));
   // first find max type of each output column.
   for (int i=0; i<data->lvalues; ++i) { // for each output column.
     tmp = VECTOR_ELT(data->valuecols, i);
@@ -401,7 +401,7 @@ static SEXP combineFactorLevels(SEXP factorLevels, SEXP target, int * factorType
   if (!isString(target)) internal_error(__func__, "expects a character target to factorize");  // # nocov
   int nrow = length(target);
   SEXP ans = PROTECT(allocVector(INTSXP, nrow));
-  SEXP *levelsRaw = (SEXP *)R_alloc(maxlevels, sizeof(SEXP));  // allocate for worst-case all-unique levels
+  SEXP *levelsRaw = (SEXP *)R_alloc(maxlevels, sizeof(*levelsRaw));  // allocate for worst-case all-unique levels
   int *ansd = INTEGER(ans);
   const SEXP *targetd = STRING_PTR_RO(target);
   savetl_init();
@@ -492,7 +492,7 @@ SEXP getvaluecols(SEXP DT, SEXP dtnames, Rboolean valfactor, Rboolean verbose, s
     data->totlen = data->nrow * data->lmax;
   }
   SEXP flevels = PROTECT(allocVector(VECSXP, data->lmax));
-  Rboolean *isordered = (Rboolean *)R_alloc(data->lmax, sizeof(Rboolean));
+  Rboolean *isordered = (Rboolean *)R_alloc(data->lmax, sizeof(*isordered));
   SEXP ansvals = PROTECT(allocVector(VECSXP, data->lvalues));
   for (int i=0; i<data->lvalues; ++i) {//for each output/value column.
     bool thisvalfactor = (data->maxtype[i] == VECSXP) ? false : valfactor;