From 119489f5e2e10562a47f5a132da7fd93e5bf9901 Mon Sep 17 00:00:00 2001
From: Michael Chirico <michaelchirico4@gmail.com>
Date: Mon, 15 Jul 2024 18:50:07 -0700
Subject: [PATCH 01/15] Consistently error when allocation request fails

---
 src/assign.c    |  2 ++
 src/bmerge.c    |  6 +++++-
 src/chmatch.c   |  4 +++-
 src/forder.c    | 43 +++++++++++++++++++++++++++++++++++--------
 src/fread.c     | 17 +++++++++++++----
 src/fsort.c     |  4 ++--
 src/fwrite.c    |  7 +++++--
 src/gsumm.c     | 16 +++++++++++-----
 src/ijoin.c     |  2 ++
 src/rbindlist.c | 17 +++++++++++++----
 src/uniqlist.c  |  7 ++++++-
 11 files changed, 97 insertions(+), 28 deletions(-)

diff --git a/src/assign.c b/src/assign.c
index 3b7ac5faa6..f7a6e6f636 100644
--- a/src/assign.c
+++ b/src/assign.c
@@ -1248,6 +1248,8 @@ void savetl_init(void) {
   saveds = (SEXP *)malloc(nalloc * sizeof(SEXP));
   savedtl = (R_len_t *)malloc(nalloc * sizeof(R_len_t));
   if (saveds==NULL || savedtl==NULL) {
+    if (saveds) free(saveds);
+    if (savedtl) free(savedtl);
     savetl_end();                                                        // # nocov
     error(_("Failed to allocate initial %d items in savetl_init"), nalloc); // # nocov
   }
diff --git a/src/bmerge.c b/src/bmerge.c
index 2ce69904e0..8eb1e0abbe 100644
--- a/src/bmerge.c
+++ b/src/bmerge.c
@@ -129,8 +129,12 @@ SEXP bmerge(SEXP idt, SEXP xdt, SEXP icolsArg, SEXP xcolsArg, SEXP isorted, SEXP
     retFirst = Calloc(anslen, int); // anslen is set above
     retLength = Calloc(anslen, int);
     retIndex = Calloc(anslen, int);
-    if (retFirst==NULL || retLength==NULL || retIndex==NULL)
+    if (retFirst==NULL || retLength==NULL || retIndex==NULL) {
+      if (retFirst) Free(retFirst);
+      if (retLength) Free(retLength);
+      if (retIndex) Free(retIndex);
       error(_("Internal error in allocating memory for non-equi join")); // # nocov
+    }
     // initialise retIndex here directly, as next loop is meant for both equi and non-equi joins
     for (int j=0; j<anslen; j++) retIndex[j] = j+1;
   } else { // equi joins (or) non-equi join but no multiple matches
diff --git a/src/chmatch.c b/src/chmatch.c
index b3ac5d818c..f54eda96a7 100644
--- a/src/chmatch.c
+++ b/src/chmatch.c
@@ -94,10 +94,12 @@ static SEXP chmatchMain(SEXP x, SEXP table, int nomatch, bool chin, bool chmatch
     //                                                                                        uniq         dups
     // 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)
-    int *counts = (int *)calloc(nuniq, sizeof(int));
     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));
     if (!counts || !map) {
+      if (counts) free(counts);
+      if (map) free(map);
       // # nocov start
       for (int i=0; i<tablelen; i++) SET_TRUELENGTH(td[i], 0);
       savetl_end();
diff --git a/src/forder.c b/src/forder.c
index 51a629284b..a197b094be 100644
--- a/src/forder.c
+++ b/src/forder.c
@@ -267,9 +267,12 @@ 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
-  if (!cradix_counts) STOP(_("Failed to alloc cradix_counts"));
   cradix_xtmp = (SEXP *)malloc(ustr_n*sizeof(SEXP));
-  if (!cradix_xtmp) STOP(_("Failed to alloc cradix_tmp"));
+  if (!cradix_counts || !cradix_xtmp) {
+    if (cradix_counts) free(cradix_counts);
+    if (cradix_xtmp) free(cradix_xtmp);
+    STOP(_("Failed to alloc cradix_counts and/or cradix_tmp"));
+  }
   cradix_r(x, n, 0);
   free(cradix_counts); cradix_counts=NULL;
   free(cradix_xtmp);   cradix_xtmp=NULL;
@@ -319,7 +322,8 @@ static void range_str(SEXP *x, int n, uint64_t *out_min, uint64_t *out_max, int
     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));
-    if (!ustr3) STOP(_("Failed to alloc ustr3 when converting strings to UTF8"));  // # nocov
+    if (!ustr3)
+      STOP(_("Failed to alloc ustr3 when converting strings to UTF8"));  // # nocov
     memcpy(ustr3, STRING_PTR(ustr2), ustr_n*sizeof(SEXP));
     // need to reset ustr_maxlen because we need ustr_maxlen for utf8 strings
     ustr_maxlen = 0;
@@ -337,7 +341,8 @@ static void range_str(SEXP *x, int n, uint64_t *out_min, uint64_t *out_max, int
     }
     // 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));
-    if (!tl) STOP(_("Failed to alloc tl when converting strings to UTF8"));  // # nocov
+    if (!tl)
+      STOP(_("Failed to alloc tl when converting strings to UTF8"));  // # nocov
     SEXP *tt = STRING_PTR(ustr2);
     for (int i=0; i<ustr_n; i++) tl[i] = TRUELENGTH(tt[i]);   // fetches the o in ustr3 into tl which is ordered by ustr
     for (int i=0; i<ustr_n; i++) SET_TRUELENGTH(ustr3[i], 0);    // reset to 0 tl of the UTF8 (and possibly non-UTF in ustr too)
@@ -719,13 +724,22 @@ SEXP forder(SEXP DT, SEXP by, SEXP retGrpArg, SEXP sortGroupsArg, SEXP ascArg, S
   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)
-  if (!TMP || !UGRP /*|| TMP%64 || UGRP%64*/) STOP(_("Failed to allocate TMP or UGRP or they weren't cache line aligned: nth=%d"), nth);
+  if (!TMP || !UGRP /*|| TMP%64 || UGRP%64*/) {
+    if (TMP) free(TMP);
+    if (UGRP) free(UGRP); // very unlikely, but knife's-edge chance some memory was freed up between the malloc() calls
+    STOP(_("Failed to allocate TMP or UGRP or they weren't cache line aligned: nth=%d"), nth);
+  }
   
   if (retgrp) {
     gs_thread = calloc(nth, sizeof(int *));     // thread private group size buffers
     gs_thread_alloc = calloc(nth, sizeof(int));
     gs_thread_n = calloc(nth, sizeof(int));
-    if (!gs_thread || !gs_thread_alloc || !gs_thread_n) STOP(_("Could not allocate (very tiny) group size thread buffers"));
+    if (!gs_thread || !gs_thread_alloc || !gs_thread_n) {
+      if (gs_thread) free(gs_thread);
+      if (gs_thread_alloc) free(gs_thread_alloc);
+      if (gs_thread_n) free(gs_thread_n);
+      STOP(_("Could not allocate (very tiny) group size thread buffers"));
+    }
   }
   if (nradix) {
     radix_r(0, nrow-1, 0);  // top level recursive call: (from, to, radix)
@@ -1045,7 +1059,12 @@ void radix_r(const int from, const int to, const int radix) {
   uint16_t *counts = calloc(nBatch*256,sizeof(uint16_t));
   uint8_t  *ugrps =  malloc(nBatch*256*sizeof(uint8_t));
   int      *ngrps =  calloc(nBatch    ,sizeof(int));
-  if (!counts || !ugrps || !ngrps) STOP(_("Failed to allocate parallel counts. my_n=%d, nBatch=%d"), my_n, nBatch);
+  if (!counts || !ugrps || !ngrps) {
+    if (counts) free(counts);
+    if (ugrps) free(ugrps);
+    if (ngrps) free(ngrps);
+    STOP(_("Failed to allocate parallel counts. my_n=%d, nBatch=%d"), my_n, nBatch);
+  }
 
   bool skip=true;
   const int n_rem = nradix-radix-1;   // how many radix are remaining after this one
@@ -1054,6 +1073,11 @@ void radix_r(const int from, const int to, const int radix) {
   {
     int     *my_otmp = malloc(batchSize * sizeof(int)); // thread-private write
     uint8_t *my_ktmp = malloc(batchSize * sizeof(uint8_t) * n_rem);
+    if (!my_otmp || !my_ktmp) {
+      if (my_otmp) free(my_otmp);
+      if (my_ktmp) free(my_ktmp);
+      STOP(_("Failed to allocate 'my_otmp' and/or 'my_ktmp' arrays (%d bytes)."), (int)(batchSize*(sizeof(int) + sizeof(uint8_t))));
+    }
     // TODO: move these up above and point restrict[me] to them. Easier to Error that way if failed to alloc.
     #pragma omp for
     for (int batch=0; batch<nBatch; batch++) {
@@ -1139,6 +1163,8 @@ void radix_r(const int from, const int to, const int radix) {
   // If skip==true and we're already done, we still need the first row of this cummulate (diff to get total group sizes) to push() or recurse below
 
   int *starts = calloc(nBatch*256, sizeof(int));  // keep starts the same shape and ugrp order as counts
+  if (!starts)
+    STOP(_("Unable to allocate 'starts' array, %d bytes."), (int)(nBatch*256*sizeof(int)));
   for (int j=0, sum=0; j<ngrp; j++) {  // iterate through columns (ngrp bytes)
     uint16_t *tmp1 = counts+ugrp[j];
     int      *tmp2 = starts+ugrp[j];
@@ -1154,7 +1180,8 @@ 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));
-    if (!TMP) STOP(_("Unable to allocate TMP for my_n=%d items in parallel batch counting"), my_n);
+    if (!TMP)
+      STOP(_("Unable to allocate TMP for my_n=%d items in parallel batch counting"), my_n);
     #pragma omp parallel for num_threads(getDTthreads(nBatch, false))
     for (int batch=0; batch<nBatch; batch++) {
       const int *restrict      my_starts = starts + batch*256;
diff --git a/src/fread.c b/src/fread.c
index 3a9f1fb268..6711245572 100644
--- a/src/fread.c
+++ b/src/fread.c
@@ -447,7 +447,8 @@ void copyFile(size_t fileSize, const char *msg, bool verbose)  // only called in
 {
   double tt = wallclock();
   mmp_copy = (char *)malloc((size_t)fileSize + 1/* extra \0 */);
-  if (!mmp_copy) STOP(_("Unable to allocate %s of contiguous virtual RAM. %s allocation."), filesize_to_str(fileSize), msg);
+  if (!mmp_copy)
+    STOP(_("Unable to allocate %s of contiguous virtual RAM. %s allocation."), filesize_to_str(fileSize), msg);
   memcpy(mmp_copy, mmp, fileSize);
   sof = mmp_copy;
   eof = (char *)mmp_copy + fileSize;
@@ -1828,7 +1829,11 @@ int freadMain(freadMainArgs _args) {
 
   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
-  if (!type || !tmpType) STOP(_("Failed to allocate 2 x %d bytes for type and tmpType: %s"), ncol, strerror(errno));
+  if (!type || !tmpType) {
+    if (type) free(type);
+    if (tmpType) free(tmpType);
+    STOP(_("Failed to allocate 2 x %d bytes for type and tmpType: %s"), ncol, strerror(errno));
+  }
 
   if (sep == ',' && dec == '\0') { // if sep=',' detected, don't attempt to detect dec [NB: . is not par of seps]
     if (verbose) {
@@ -2074,7 +2079,8 @@ int freadMain(freadMainArgs _args) {
     colNames = NULL;  // userOverride will assign V1, V2, etc
   } else {
     colNames = (lenOff*) calloc((size_t)ncol, sizeof(lenOff));
-    if (!colNames) STOP(_("Unable to allocate %d*%d bytes for column name pointers: %s"), ncol, sizeof(lenOff), strerror(errno));
+    if (!colNames)
+      STOP(_("Unable to allocate %d*%d bytes for column name pointers: %s"), ncol, sizeof(lenOff), strerror(errno));
     if (sep==' ') while (*ch==' ') ch++;
     void *targets[9] = {NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, colNames + autoFirstColName};
     FieldParseContext fctx = {
@@ -2128,7 +2134,8 @@ int freadMain(freadMainArgs _args) {
   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
-  if (!size) STOP(_("Failed to allocate %d bytes for size array: %s"), ncol, strerror(errno));
+  if (!size)
+    STOP(_("Failed to allocate %d bytes for size array: %s"), ncol, strerror(errno));
   nStringCols = 0;
   nNonStringCols = 0;
   for (int j=0; j<ncol; j++) {
@@ -2567,6 +2574,8 @@ int freadMain(freadMainArgs _args) {
       DTPRINT(_("  Dropping %d overallocated columns\n"), ndropFill);
     }
     dropFill = (int *)malloc((size_t)ndropFill * sizeof(int));
+    if (!dropFill)
+      STOP(_("Failed to allocate %d bytes for dropFill."), (int)(ndropFill * sizeof(int)));
     int i=0;
     for (int j=max_col; j<ncol; ++j) {
       type[j] = CT_DROP;
diff --git a/src/fsort.c b/src/fsort.c
index 2618ec577b..3aefe9d7a0 100644
--- a/src/fsort.c
+++ b/src/fsort.c
@@ -256,7 +256,7 @@ SEXP fsort(SEXP x, SEXP verboseArg) {
       // each thread has its own small stack of counts
       // don't use VLAs here: perhaps too big for stack yes but more that VLAs apparently fail with schedule(dynamic)
       uint64_t *restrict mycounts = calloc((toBit/8 + 1)*256, sizeof(uint64_t));
-      if (mycounts==NULL) {
+      if (!mycounts) {
         failed=true; alloc_fail=true;  // # nocov
       }
       double *restrict myworking = NULL;
@@ -283,7 +283,7 @@ SEXP fsort(SEXP x, SEXP verboseArg) {
 
         if (myworking==NULL) {
           myworking = malloc(thisN * sizeof(double));
-          if (myworking==NULL) {
+          if (!myworking) {
             failed=true; alloc_fail=true; continue;  // # nocov
           }
           myfirstmsb = msb;
diff --git a/src/fwrite.c b/src/fwrite.c
index 40831089c7..dacfc7e11f 100644
--- a/src/fwrite.c
+++ b/src/fwrite.c
@@ -715,7 +715,8 @@ void fwriteMain(fwriteMainArgs args)
   }
   if (headerLen) {
     char *buff = malloc(headerLen);
-    if (!buff) STOP(_("Unable to allocate %zu MiB for header: %s"), headerLen / 1024 / 1024, strerror(errno));
+    if (!buff)
+      STOP(_("Unable to allocate %zu MiB for header: %s"), headerLen / 1024 / 1024, strerror(errno));
     char *ch = buff;
     if (args.bom) {*ch++=(char)0xEF; *ch++=(char)0xBB; *ch++=(char)0xBF; }  // 3 appears above (search for "bom")
     memcpy(ch, args.yaml, yamlLen);
@@ -830,8 +831,8 @@ void fwriteMain(fwriteMainArgs args)
   }
   char *zbuffPool = NULL;
   if (args.is_gzip) {
-    zbuffPool = malloc(nth*(size_t)zbuffSize);
 #ifndef NOZLIB
+    zbuffPool = malloc(nth*(size_t)zbuffSize);
     if (!zbuffPool) {
       // # nocov start
       free(buffPool);
@@ -986,7 +987,9 @@ void fwriteMain(fwriteMainArgs args)
     }
   }
   free(buffPool);
+#ifndef NOZLIB
   free(zbuffPool);
+#endif
 
   // Finished parallel region and can call R API safely now.
   if (hasPrinted) {
diff --git a/src/gsumm.c b/src/gsumm.c
index 52dca111b6..b95c442657 100644
--- a/src/gsumm.c
+++ b/src/gsumm.c
@@ -113,7 +113,11 @@ SEXP gforce(SEXP env, SEXP jsub, SEXP o, SEXP f, SEXP l, SEXP irowsArg) {
     //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
-    if (!counts || !TMP ) error(_("Internal error: Failed to allocate counts or TMP when assigning g in gforce"));
+    if (!counts || !TMP ) {
+      if (counts) free(counts);
+      if (TMP) free(TMP);
+      error(_("Internal error: Failed to allocate counts or TMP when assigning g in gforce"));
+    }
     #pragma omp parallel for num_threads(getDTthreads(nBatch, false))   // schedule(dynamic,1)
     for (int b=0; b<nBatch; b++) {
       const int howMany = b==nBatch-1 ? lastBatchSize : batchSize;
@@ -617,7 +621,8 @@ SEXP gmean(SEXP x, SEXP narmArg)
     } else {
       // narm==true and anyNA==true
       int *restrict nna_counts = calloc(ngrp, sizeof(int));
-      if (!nna_counts) error(_("Unable to allocate %d * %zu bytes for non-NA counts in gmean na.rm=TRUE"), ngrp, sizeof(int));
+      if (!nna_counts)
+        error(_("Unable to allocate %d * %zu bytes for non-NA counts in gmean na.rm=TRUE"), ngrp, sizeof(int));
       #pragma omp parallel for num_threads(getDTthreads(highSize, false))
       for (int h=0; h<highSize; h++) {
           double *restrict _ans = ansp + (h<<bitshift);
@@ -672,8 +677,8 @@ SEXP gmean(SEXP x, SEXP narmArg)
       int *restrict nna_counts_i = calloc(ngrp, sizeof(int));
       if (!nna_counts_r || !nna_counts_i) {
         // # nocov start
-        free(nna_counts_r);  // free(NULL) is allowed and does nothing. Avoids repeating the error() call here.
-        free(nna_counts_i);
+        if (nna_counts_r) free(nna_counts_r);
+        if (nna_counts_i) free(nna_counts_i);
         error(_("Unable to allocate %d * %zu bytes for non-NA counts in gmean na.rm=TRUE"), ngrp, sizeof(int));
         // # nocov end
       }
@@ -1117,7 +1122,8 @@ SEXP gprod(SEXP x, SEXP narmArg) {
   //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));
-  if (!s) error(_("Unable to allocate %d * %zu bytes for gprod"), ngrp, sizeof(long double));
+  if (!s)
+    error(_("Unable to allocate %d * %zu bytes for gprod"), ngrp, sizeof(long double));
   for (int i=0; i<ngrp; ++i) s[i] = 1.0;
   switch(TYPEOF(x)) {
   case LGLSXP: case INTSXP: {
diff --git a/src/ijoin.c b/src/ijoin.c
index b4f0a4b081..129cda9114 100644
--- a/src/ijoin.c
+++ b/src/ijoin.c
@@ -142,6 +142,8 @@ SEXP lookup(SEXP ux, SEXP xlen, SEXP indices, SEXP gaps, SEXP overlaps, SEXP mul
   // generate lookup
   start = clock();
   idx = Calloc(uxrows, R_len_t); // resets bits, =0
+  if (!idx)
+    error(_("Failed to allocate %d bytes for idx.", (int)(uxrows * sizeof(R_len_t))));
   switch (type) {
   case ANY: case START: case END: case WITHIN:
     for (int i=0; i<xrows; ++i) {
diff --git a/src/rbindlist.c b/src/rbindlist.c
index 5fd4cf7000..fef443ad17 100644
--- a/src/rbindlist.c
+++ b/src/rbindlist.c
@@ -67,7 +67,8 @@ SEXP rbindlist(SEXP l, SEXP usenamesArg, SEXP fillArg, SEXP idcolArg)
     // when use.names==NA we also proceed here as if use.names was TRUE to save new code and then check afterwards the map is 1:ncol for every item
     // first find number of unique column names present; i.e. length(unique(unlist(lapply(l,names))))
     SEXP *uniq = (SEXP *)malloc(upperBoundUniqueNames * sizeof(SEXP));  // upperBoundUniqueNames was initialized with 1 to ensure this is defined (otherwise 0 when no item has names)
-    if (!uniq) error(_("Failed to allocate upper bound of %"PRId64" unique column names [sum(lapply(l,ncol))]"), (int64_t)upperBoundUniqueNames);
+    if (!uniq)
+      error(_("Failed to allocate upper bound of %"PRId64" unique column names [sum(lapply(l,ncol))]"), (int64_t)upperBoundUniqueNames);
     savetl_init();
     int nuniq=0;
     for (int i=0; i<LENGTH(l); i++) {
@@ -96,7 +97,9 @@ SEXP rbindlist(SEXP l, SEXP usenamesArg, SEXP fillArg, SEXP idcolArg)
     if (!counts || !maxdup) {
       // # nocov start
       for (int i=0; i<nuniq; ++i) SET_TRUELENGTH(uniq[i], 0);
-      free(uniq); free(counts); free(maxdup);
+      free(uniq);
+      if (counts) free(counts);
+      if (maxdup) free(maxdup);
       savetl_end();
       error(_("Failed to allocate nuniq=%d items working memory in rbindlist.c"), nuniq);
       // # nocov end
@@ -130,7 +133,10 @@ SEXP rbindlist(SEXP l, SEXP usenamesArg, SEXP fillArg, SEXP idcolArg)
     if (!colMapRaw || !uniqMap || !dupLink) {
       // # nocov start
       for (int i=0; i<nuniq; ++i) SET_TRUELENGTH(uniq[i], 0);
-      free(uniq); free(counts); free(colMapRaw); free(uniqMap); free(dupLink);
+      free(uniq); free(counts);
+      if (colMapRaw) free(colMapRaw);
+      if (uniqMap) free(uniqMap);
+      if (dupLink) free(dupLink);
       savetl_end();
       error(_("Failed to allocate ncol=%d items working memory in rbindlist.c"), ncol);
       // # nocov end
@@ -359,7 +365,10 @@ SEXP rbindlist(SEXP l, SEXP usenamesArg, SEXP fillArg, SEXP idcolArg)
         const SEXP *sd = STRING_PTR(longestLevels);
         nLevel = allocLevel = longestLen;
         levelsRaw = (SEXP *)malloc(nLevel * sizeof(SEXP));
-        if (!levelsRaw) { savetl_end(); error(_("Failed to allocate working memory for %d ordered factor levels of result column %d"), nLevel, idcol+j+1); }
+        if (!levelsRaw) {
+          savetl_end();
+          error(_("Failed to allocate working memory for %d ordered factor levels of result column %d"), nLevel, idcol+j+1);
+        }
         for (int k=0; k<longestLen; ++k) {
           SEXP s = sd[k];
           if (TRUELENGTH(s)>0) savetl(s);
diff --git a/src/uniqlist.c b/src/uniqlist.c
index 48da706a2c..c67d307774 100644
--- a/src/uniqlist.c
+++ b/src/uniqlist.c
@@ -23,6 +23,8 @@ SEXP uniqlist(SEXP l, SEXP order)
   SEXP v, ans;
   R_len_t len, thisi, previ, isize=1000;
   int *iidx = Calloc(isize, int); // for 'idx'
+  if (!iidx)
+    error(_("Failed to allocate %d bytes for 'iidx'."), (int)(isize * sizeof(int)));
   len = 1;
   iidx[0] = 1; // first row is always the first of the first group
 
@@ -259,7 +261,10 @@ SEXP nestedid(SEXP l, SEXP cols, SEXP order, SEXP grps, SEXP resetvals, SEXP mul
   R_len_t nrows = length(VECTOR_ELT(l,0)), ncols = length(cols);
   if (nrows==0) return(allocVector(INTSXP, 0));
   R_len_t thisi, previ, ansgrpsize=1000, nansgrp=0;
-  R_len_t *ansgrp = Calloc(ansgrpsize, R_len_t), starts, grplen; // #3401 fix. Needs to be Calloc due to Realloc below .. else segfaults.
+  R_len_t *ansgrp = Calloc(ansgrpsize, R_len_t);
+  if (!ansgrp)
+    error(_("Failed to allocate %d bytes for '%s'."), (int)(ansgrpsize * sizeof(R_len_t)), "ansgrpsize");
+  R_len_t starts, grplen; // #3401 fix. Needs to be Calloc due to Realloc below .. else segfaults.
   R_len_t ngrps = length(grps);
   bool *i64 = (bool *)R_alloc(ncols, sizeof(bool));
   if (ngrps==0) error(_("Internal error: nrows[%d]>0 but ngrps==0"), nrows); // # nocov

From 97e684bce7fb3688d253e7311d132303ab1d5405 Mon Sep 17 00:00:00 2001
From: Michael Chirico <michaelchirico4@gmail.com>
Date: Mon, 15 Jul 2024 18:56:56 -0700
Subject: [PATCH 02/15] Just free() unconditionally

---
 src/assign.c    |  3 +--
 src/bmerge.c    |  4 +---
 src/chmatch.c   |  3 +--
 src/forder.c    | 17 +++++------------
 src/fread.c     |  3 +--
 src/gsumm.c     |  6 ++----
 src/rbindlist.c |  9 ++-------
 7 files changed, 13 insertions(+), 32 deletions(-)

diff --git a/src/assign.c b/src/assign.c
index f7a6e6f636..b475c6f638 100644
--- a/src/assign.c
+++ b/src/assign.c
@@ -1248,8 +1248,7 @@ void savetl_init(void) {
   saveds = (SEXP *)malloc(nalloc * sizeof(SEXP));
   savedtl = (R_len_t *)malloc(nalloc * sizeof(R_len_t));
   if (saveds==NULL || savedtl==NULL) {
-    if (saveds) free(saveds);
-    if (savedtl) free(savedtl);
+    free(saveds); free(savedtl);
     savetl_end();                                                        // # nocov
     error(_("Failed to allocate initial %d items in savetl_init"), nalloc); // # nocov
   }
diff --git a/src/bmerge.c b/src/bmerge.c
index 8eb1e0abbe..98a4b7b707 100644
--- a/src/bmerge.c
+++ b/src/bmerge.c
@@ -130,9 +130,7 @@ SEXP bmerge(SEXP idt, SEXP xdt, SEXP icolsArg, SEXP xcolsArg, SEXP isorted, SEXP
     retLength = Calloc(anslen, int);
     retIndex = Calloc(anslen, int);
     if (retFirst==NULL || retLength==NULL || retIndex==NULL) {
-      if (retFirst) Free(retFirst);
-      if (retLength) Free(retLength);
-      if (retIndex) Free(retIndex);
+      Free(retFirst); Free(retLength); Free(retIndex);
       error(_("Internal error in allocating memory for non-equi join")); // # nocov
     }
     // initialise retIndex here directly, as next loop is meant for both equi and non-equi joins
diff --git a/src/chmatch.c b/src/chmatch.c
index f54eda96a7..878699e6ae 100644
--- a/src/chmatch.c
+++ b/src/chmatch.c
@@ -98,8 +98,7 @@ static SEXP chmatchMain(SEXP x, SEXP table, int nomatch, bool chin, bool chmatch
     int *counts = (int *)calloc(nuniq, sizeof(int));
     int *map =    (int *)calloc(mapsize, sizeof(int));
     if (!counts || !map) {
-      if (counts) free(counts);
-      if (map) free(map);
+      free(counts); free(map);
       // # nocov start
       for (int i=0; i<tablelen; i++) SET_TRUELENGTH(td[i], 0);
       savetl_end();
diff --git a/src/forder.c b/src/forder.c
index a197b094be..e1b60976ff 100644
--- a/src/forder.c
+++ b/src/forder.c
@@ -269,8 +269,7 @@ 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));
   if (!cradix_counts || !cradix_xtmp) {
-    if (cradix_counts) free(cradix_counts);
-    if (cradix_xtmp) free(cradix_xtmp);
+    free(cradix_counts); free(cradix_xtmp);
     STOP(_("Failed to alloc cradix_counts and/or cradix_tmp"));
   }
   cradix_r(x, n, 0);
@@ -725,8 +724,7 @@ SEXP forder(SEXP DT, SEXP by, SEXP retGrpArg, SEXP sortGroupsArg, SEXP ascArg, S
   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)
   if (!TMP || !UGRP /*|| TMP%64 || UGRP%64*/) {
-    if (TMP) free(TMP);
-    if (UGRP) free(UGRP); // very unlikely, but knife's-edge chance some memory was freed up between the malloc() calls
+    free(TMP); free(UGRP);
     STOP(_("Failed to allocate TMP or UGRP or they weren't cache line aligned: nth=%d"), nth);
   }
   
@@ -735,9 +733,7 @@ SEXP forder(SEXP DT, SEXP by, SEXP retGrpArg, SEXP sortGroupsArg, SEXP ascArg, S
     gs_thread_alloc = calloc(nth, sizeof(int));
     gs_thread_n = calloc(nth, sizeof(int));
     if (!gs_thread || !gs_thread_alloc || !gs_thread_n) {
-      if (gs_thread) free(gs_thread);
-      if (gs_thread_alloc) free(gs_thread_alloc);
-      if (gs_thread_n) free(gs_thread_n);
+      free(gs_thread); free(gs_thread_alloc); free(gs_thread_n);
       STOP(_("Could not allocate (very tiny) group size thread buffers"));
     }
   }
@@ -1060,9 +1056,7 @@ void radix_r(const int from, const int to, const int radix) {
   uint8_t  *ugrps =  malloc(nBatch*256*sizeof(uint8_t));
   int      *ngrps =  calloc(nBatch    ,sizeof(int));
   if (!counts || !ugrps || !ngrps) {
-    if (counts) free(counts);
-    if (ugrps) free(ugrps);
-    if (ngrps) free(ngrps);
+    free(counts); free(ugrps); free(ngrps);
     STOP(_("Failed to allocate parallel counts. my_n=%d, nBatch=%d"), my_n, nBatch);
   }
 
@@ -1074,8 +1068,7 @@ void radix_r(const int from, const int to, const int radix) {
     int     *my_otmp = malloc(batchSize * sizeof(int)); // thread-private write
     uint8_t *my_ktmp = malloc(batchSize * sizeof(uint8_t) * n_rem);
     if (!my_otmp || !my_ktmp) {
-      if (my_otmp) free(my_otmp);
-      if (my_ktmp) free(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))));
     }
     // TODO: move these up above and point restrict[me] to them. Easier to Error that way if failed to alloc.
diff --git a/src/fread.c b/src/fread.c
index 6711245572..46a28eca5c 100644
--- a/src/fread.c
+++ b/src/fread.c
@@ -1830,8 +1830,7 @@ int freadMain(freadMainArgs _args) {
   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
   if (!type || !tmpType) {
-    if (type) free(type);
-    if (tmpType) free(tmpType);
+    free(type); free(tmpType);
     STOP(_("Failed to allocate 2 x %d bytes for type and tmpType: %s"), ncol, strerror(errno));
   }
 
diff --git a/src/gsumm.c b/src/gsumm.c
index b95c442657..7607f42588 100644
--- a/src/gsumm.c
+++ b/src/gsumm.c
@@ -114,8 +114,7 @@ SEXP gforce(SEXP env, SEXP jsub, SEXP o, SEXP f, SEXP l, SEXP irowsArg) {
     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
     if (!counts || !TMP ) {
-      if (counts) free(counts);
-      if (TMP) free(TMP);
+      free(counts); free(TMP);
       error(_("Internal error: Failed to allocate counts or TMP when assigning g in gforce"));
     }
     #pragma omp parallel for num_threads(getDTthreads(nBatch, false))   // schedule(dynamic,1)
@@ -677,8 +676,7 @@ SEXP gmean(SEXP x, SEXP narmArg)
       int *restrict nna_counts_i = calloc(ngrp, sizeof(int));
       if (!nna_counts_r || !nna_counts_i) {
         // # nocov start
-        if (nna_counts_r) free(nna_counts_r);
-        if (nna_counts_i) free(nna_counts_i);
+        free(nna_counts_r); free(nna_counts_i);
         error(_("Unable to allocate %d * %zu bytes for non-NA counts in gmean na.rm=TRUE"), ngrp, sizeof(int));
         // # nocov end
       }
diff --git a/src/rbindlist.c b/src/rbindlist.c
index fef443ad17..24a785bde8 100644
--- a/src/rbindlist.c
+++ b/src/rbindlist.c
@@ -97,9 +97,7 @@ SEXP rbindlist(SEXP l, SEXP usenamesArg, SEXP fillArg, SEXP idcolArg)
     if (!counts || !maxdup) {
       // # nocov start
       for (int i=0; i<nuniq; ++i) SET_TRUELENGTH(uniq[i], 0);
-      free(uniq);
-      if (counts) free(counts);
-      if (maxdup) free(maxdup);
+      free(uniq); free(counts); free(maxdup);
       savetl_end();
       error(_("Failed to allocate nuniq=%d items working memory in rbindlist.c"), nuniq);
       // # nocov end
@@ -133,10 +131,7 @@ SEXP rbindlist(SEXP l, SEXP usenamesArg, SEXP fillArg, SEXP idcolArg)
     if (!colMapRaw || !uniqMap || !dupLink) {
       // # nocov start
       for (int i=0; i<nuniq; ++i) SET_TRUELENGTH(uniq[i], 0);
-      free(uniq); free(counts);
-      if (colMapRaw) free(colMapRaw);
-      if (uniqMap) free(uniqMap);
-      if (dupLink) free(dupLink);
+      free(uniq); free(counts); free(colMapRaw); free(uniqMap); free(dupLink);
       savetl_end();
       error(_("Failed to allocate ncol=%d items working memory in rbindlist.c"), ncol);
       // # nocov end

From 72e3045f22e23fd9db30cfe7373d0d21829bf243 Mon Sep 17 00:00:00 2001
From: Michael Chirico <michaelchirico4@gmail.com>
Date: Mon, 15 Jul 2024 19:01:35 -0700
Subject: [PATCH 03/15] paren mismatch

---
 src/ijoin.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/ijoin.c b/src/ijoin.c
index 129cda9114..da7c5ab765 100644
--- a/src/ijoin.c
+++ b/src/ijoin.c
@@ -143,7 +143,7 @@ SEXP lookup(SEXP ux, SEXP xlen, SEXP indices, SEXP gaps, SEXP overlaps, SEXP mul
   start = clock();
   idx = Calloc(uxrows, R_len_t); // resets bits, =0
   if (!idx)
-    error(_("Failed to allocate %d bytes for idx.", (int)(uxrows * sizeof(R_len_t))));
+    error(_("Failed to allocate %d bytes for idx."), (int)(uxrows * sizeof(R_len_t)));
   switch (type) {
   case ANY: case START: case END: case WITHIN:
     for (int i=0; i<xrows; ++i) {

From 97a1cbfcba00fe7c0312fe451aca4784b36044ba Mon Sep 17 00:00:00 2001
From: Michael Chirico <michaelchirico4@gmail.com>
Date: Mon, 15 Jul 2024 19:06:49 -0700
Subject: [PATCH 04/15] More consistency for easier i18n

---
 src/forder.c   | 2 +-
 src/fread.c    | 4 ++--
 src/ijoin.c    | 2 +-
 src/uniqlist.c | 2 +-
 4 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/forder.c b/src/forder.c
index e1b60976ff..70b7070e3c 100644
--- a/src/forder.c
+++ b/src/forder.c
@@ -1157,7 +1157,7 @@ void radix_r(const int from, const int to, const int radix) {
 
   int *starts = calloc(nBatch*256, sizeof(int));  // keep starts the same shape and ugrp order as counts
   if (!starts)
-    STOP(_("Unable to allocate 'starts' array, %d bytes."), (int)(nBatch*256*sizeof(int)));
+    STOP(_("Failed to allocate %d bytes for '%s'."), (int)(nBatch*256*sizeof(int)), "starts");
   for (int j=0, sum=0; j<ngrp; j++) {  // iterate through columns (ngrp bytes)
     uint16_t *tmp1 = counts+ugrp[j];
     int      *tmp2 = starts+ugrp[j];
diff --git a/src/fread.c b/src/fread.c
index 46a28eca5c..5a14253e0f 100644
--- a/src/fread.c
+++ b/src/fread.c
@@ -2134,7 +2134,7 @@ int freadMain(freadMainArgs _args) {
   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
   if (!size)
-    STOP(_("Failed to allocate %d bytes for size array: %s"), ncol, strerror(errno));
+    STOP(_("Failed to allocate %d bytes for '%s': %s"), (int)(ncol * sizeof(int8_t)), "size", strerror(errno));
   nStringCols = 0;
   nNonStringCols = 0;
   for (int j=0; j<ncol; j++) {
@@ -2574,7 +2574,7 @@ int freadMain(freadMainArgs _args) {
     }
     dropFill = (int *)malloc((size_t)ndropFill * sizeof(int));
     if (!dropFill)
-      STOP(_("Failed to allocate %d bytes for dropFill."), (int)(ndropFill * sizeof(int)));
+      STOP(_("Failed to allocate %d bytes for '%s'."), (int)(ndropFill * sizeof(int)), "dropFill");
     int i=0;
     for (int j=max_col; j<ncol; ++j) {
       type[j] = CT_DROP;
diff --git a/src/ijoin.c b/src/ijoin.c
index da7c5ab765..d9d0b427b2 100644
--- a/src/ijoin.c
+++ b/src/ijoin.c
@@ -143,7 +143,7 @@ SEXP lookup(SEXP ux, SEXP xlen, SEXP indices, SEXP gaps, SEXP overlaps, SEXP mul
   start = clock();
   idx = Calloc(uxrows, R_len_t); // resets bits, =0
   if (!idx)
-    error(_("Failed to allocate %d bytes for idx."), (int)(uxrows * sizeof(R_len_t)));
+    error(_("Failed to allocate %d bytes for '%s'."), (int)(uxrows * sizeof(R_len_t)), "idx");
   switch (type) {
   case ANY: case START: case END: case WITHIN:
     for (int i=0; i<xrows; ++i) {
diff --git a/src/uniqlist.c b/src/uniqlist.c
index c67d307774..66ea52372b 100644
--- a/src/uniqlist.c
+++ b/src/uniqlist.c
@@ -24,7 +24,7 @@ SEXP uniqlist(SEXP l, SEXP order)
   R_len_t len, thisi, previ, isize=1000;
   int *iidx = Calloc(isize, int); // for 'idx'
   if (!iidx)
-    error(_("Failed to allocate %d bytes for 'iidx'."), (int)(isize * sizeof(int)));
+    error(_("Failed to allocate %d bytes for '%s'."), (int)(isize * sizeof(int)), "iidx");
   len = 1;
   iidx[0] = 1; // first row is always the first of the first group
 

From 3ae19456bf20e76b316fec0fa6ff33c654083a92 Mon Sep 17 00:00:00 2001
From: Michael Chirico <michaelchirico4@gmail.com>
Date: Mon, 15 Jul 2024 19:13:16 -0700
Subject: [PATCH 05/15] Calloc() very intentionally _does not need_ error
 checking (handled by R)

---
 src/bmerge.c   | 4 ----
 src/ijoin.c    | 2 --
 src/uniqlist.c | 4 ----
 3 files changed, 10 deletions(-)

diff --git a/src/bmerge.c b/src/bmerge.c
index 98a4b7b707..6cb20c683f 100644
--- a/src/bmerge.c
+++ b/src/bmerge.c
@@ -129,10 +129,6 @@ SEXP bmerge(SEXP idt, SEXP xdt, SEXP icolsArg, SEXP xcolsArg, SEXP isorted, SEXP
     retFirst = Calloc(anslen, int); // anslen is set above
     retLength = Calloc(anslen, int);
     retIndex = Calloc(anslen, int);
-    if (retFirst==NULL || retLength==NULL || retIndex==NULL) {
-      Free(retFirst); Free(retLength); Free(retIndex);
-      error(_("Internal error in allocating memory for non-equi join")); // # nocov
-    }
     // initialise retIndex here directly, as next loop is meant for both equi and non-equi joins
     for (int j=0; j<anslen; j++) retIndex[j] = j+1;
   } else { // equi joins (or) non-equi join but no multiple matches
diff --git a/src/ijoin.c b/src/ijoin.c
index d9d0b427b2..b4f0a4b081 100644
--- a/src/ijoin.c
+++ b/src/ijoin.c
@@ -142,8 +142,6 @@ SEXP lookup(SEXP ux, SEXP xlen, SEXP indices, SEXP gaps, SEXP overlaps, SEXP mul
   // generate lookup
   start = clock();
   idx = Calloc(uxrows, R_len_t); // resets bits, =0
-  if (!idx)
-    error(_("Failed to allocate %d bytes for '%s'."), (int)(uxrows * sizeof(R_len_t)), "idx");
   switch (type) {
   case ANY: case START: case END: case WITHIN:
     for (int i=0; i<xrows; ++i) {
diff --git a/src/uniqlist.c b/src/uniqlist.c
index 66ea52372b..42882b39cd 100644
--- a/src/uniqlist.c
+++ b/src/uniqlist.c
@@ -23,8 +23,6 @@ SEXP uniqlist(SEXP l, SEXP order)
   SEXP v, ans;
   R_len_t len, thisi, previ, isize=1000;
   int *iidx = Calloc(isize, int); // for 'idx'
-  if (!iidx)
-    error(_("Failed to allocate %d bytes for '%s'."), (int)(isize * sizeof(int)), "iidx");
   len = 1;
   iidx[0] = 1; // first row is always the first of the first group
 
@@ -262,8 +260,6 @@ SEXP nestedid(SEXP l, SEXP cols, SEXP order, SEXP grps, SEXP resetvals, SEXP mul
   if (nrows==0) return(allocVector(INTSXP, 0));
   R_len_t thisi, previ, ansgrpsize=1000, nansgrp=0;
   R_len_t *ansgrp = Calloc(ansgrpsize, R_len_t);
-  if (!ansgrp)
-    error(_("Failed to allocate %d bytes for '%s'."), (int)(ansgrpsize * sizeof(R_len_t)), "ansgrpsize");
   R_len_t starts, grplen; // #3401 fix. Needs to be Calloc due to Realloc below .. else segfaults.
   R_len_t ngrps = length(grps);
   bool *i64 = (bool *)R_alloc(ncols, sizeof(bool));

From b2b8cb801eaf98edf9f713f88161dd7a0ca5be2b Mon Sep 17 00:00:00 2001
From: Michael Chirico <michaelchirico4@gmail.com>
Date: Mon, 15 Jul 2024 19:16:12 -0700
Subject: [PATCH 06/15] Smaller diff thanks to Calloc() behavior

---
 src/uniqlist.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/uniqlist.c b/src/uniqlist.c
index 42882b39cd..48da706a2c 100644
--- a/src/uniqlist.c
+++ b/src/uniqlist.c
@@ -259,8 +259,7 @@ SEXP nestedid(SEXP l, SEXP cols, SEXP order, SEXP grps, SEXP resetvals, SEXP mul
   R_len_t nrows = length(VECTOR_ELT(l,0)), ncols = length(cols);
   if (nrows==0) return(allocVector(INTSXP, 0));
   R_len_t thisi, previ, ansgrpsize=1000, nansgrp=0;
-  R_len_t *ansgrp = Calloc(ansgrpsize, R_len_t);
-  R_len_t starts, grplen; // #3401 fix. Needs to be Calloc due to Realloc below .. else segfaults.
+  R_len_t *ansgrp = Calloc(ansgrpsize, R_len_t), starts, grplen; // #3401 fix. Needs to be Calloc due to Realloc below .. else segfaults.
   R_len_t ngrps = length(grps);
   bool *i64 = (bool *)R_alloc(ncols, sizeof(bool));
   if (ngrps==0) error(_("Internal error: nrows[%d]>0 but ngrps==0"), nrows); // # nocov

From 3fa2bdb77ac7fac862471e0552acee7267421097 Mon Sep 17 00:00:00 2001
From: Michael Chirico <chiricom@google.com>
Date: Tue, 16 Jul 2024 05:57:51 +0000
Subject: [PATCH 07/15] consistency: !x, not x==NULL

---
 src/assign.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/assign.c b/src/assign.c
index b475c6f638..6eb0866e43 100644
--- a/src/assign.c
+++ b/src/assign.c
@@ -1247,7 +1247,7 @@ void savetl_init(void) {
   nalloc = 100;
   saveds = (SEXP *)malloc(nalloc * sizeof(SEXP));
   savedtl = (R_len_t *)malloc(nalloc * sizeof(R_len_t));
-  if (saveds==NULL || savedtl==NULL) {
+  if (!saveds || !savedtl) {
     free(saveds); free(savedtl);
     savetl_end();                                                        // # nocov
     error(_("Failed to allocate initial %d items in savetl_init"), nalloc); // # nocov

From 4d02d0cd7cc69c8136ae9bf494113dc9f3be0eb0 Mon Sep 17 00:00:00 2001
From: Michael Chirico <chiricom@google.com>
Date: Tue, 16 Jul 2024 06:03:15 +0000
Subject: [PATCH 08/15] initialize alloc_linter() checker

---
 .ci/linters/alloc_linter.R | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)
 create mode 100644 .ci/linters/alloc_linter.R

diff --git a/.ci/linters/alloc_linter.R b/.ci/linters/alloc_linter.R
new file mode 100644
index 0000000000..a94bc27a92
--- /dev/null
+++ b/.ci/linters/alloc_linter.R
@@ -0,0 +1,36 @@
+# Ensure that we check the result of malloc()/calloc() for success
+# More specifically, given that this is an AST-ignorant checker,
+#   1. Find groups of malloc()/calloc() calls
+#   2. Check the next line for a check like 'if (!x || !y)'
+alloc_linter = function(c_file) {
+  lines <- readLines(c_file)
+  # Be a bit more precise to avoid mentions in comments
+  alloc_lines <- grep(R"{=\s*([(]\w+\s*[*][)])?[mc]alloc[(]}", lines)
+  if (!length(alloc_lines)) return()
+  # int *tmp=(int*)malloc(...); or just int tmp=malloc(...);
+  alloc_keys <- lines[alloc_lines] |>
+    strsplit(R"(\s*=\s*)") |>
+    vapply(head, 1L, FUN.VALUE="") |>
+    trimws() |>
+    # just normalize the more exotic assignments, namely 'type *restrict key = ...'
+    gsub(pattern = "[*]\\s*(restrict\\s*)?", replacement = "*") |>
+    strsplit("*", fixed=TRUE) |>
+    vapply(tail, 1L, FUN.VALUE="")
+  alloc_grp_id <- cumsum(c(TRUE, diff(alloc_lines) != 1L))
+
+  # execute by group
+  tapply(seq_along(alloc_lines), alloc_grp_id, function(grp_idx) {
+    keys_regex <- paste0("\\s*!\\s*", alloc_keys[grp_idx], "\\s*", collapse = "[|][|]")
+    check_regex <- paste0("if\\s*\\(", keys_regex)
+    check_line <- lines[alloc_lines[tail(grp_idx, 1L)] + 1L]
+    # Rarely (once in fread.c as of initialization), error checking is handled
+    #   but not immediately, so use 'NOCHECK' to escape.
+    if (!grepl(check_regex, check_line) && !grepl("NOCHECK", check_line, fixed=TRUE)) {
+      bad_lines_idx <- seq(alloc_lines[grp_idx[1L]], length.out=length(grp_idx)+1L)
+      cat("FILE: ", c_file, "; LINES: ", head(bad_lines_idx, 1L), "-", tail(bad_lines_idx, 1L), "\n", sep="")
+      writeLines(lines[bad_lines_idx])
+      cat(strrep("-", max(nchar(lines[bad_lines_idx]))), "\n", sep="")
+      stop("Expected the malloc()/calloc() usage above to be followed immediately by error checking.", call.=FALSE)
+    }
+  })
+}

From 1d9389a6cf33f310eead0af62a14284f10e406fb Mon Sep 17 00:00:00 2001
From: Michael Chirico <chiricom@google.com>
Date: Tue, 16 Jul 2024 06:03:26 +0000
Subject: [PATCH 09/15] one valid //NOCHECK usage

---
 src/fread.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/fread.c b/src/fread.c
index 5a14253e0f..e301e8cd1e 100644
--- a/src/fread.c
+++ b/src/fread.c
@@ -2254,6 +2254,7 @@ int freadMain(freadMainArgs _args) {
       .buff8 = malloc(rowSize8 * myBuffRows + 8),
       .buff4 = malloc(rowSize4 * myBuffRows + 4),
       .buff1 = malloc(rowSize1 * myBuffRows + 1),
+      // NOCHECK
       .rowSize8 = rowSize8,
       .rowSize4 = rowSize4,
       .rowSize1 = rowSize1,

From a8d1004739822cfd7f4ee4ccf5e7d29056026766 Mon Sep 17 00:00:00 2001
From: Michael Chirico <chiricom@google.com>
Date: Tue, 16 Jul 2024 06:03:42 +0000
Subject: [PATCH 10/15] check in CI

---
 .../workflows/{lint.yaml => code-quality.yaml}    | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)
 rename .github/workflows/{lint.yaml => code-quality.yaml} (65%)

diff --git a/.github/workflows/lint.yaml b/.github/workflows/code-quality.yaml
similarity index 65%
rename from .github/workflows/lint.yaml
rename to .github/workflows/code-quality.yaml
index d5a4ef4662..20222b606c 100644
--- a/.github/workflows/lint.yaml
+++ b/.github/workflows/code-quality.yaml
@@ -6,10 +6,10 @@ on:
     branches:
      - master
 
-name: lint
+name: code-quality
 
 jobs:
-  lint:
+  lint-r:
     runs-on: ubuntu-latest
     env:
       GITHUB_PAT: ${{ secrets.GITHUB_TOKEN }}
@@ -33,3 +33,14 @@ jobs:
         env:
           LINTR_ERROR_ON_LINT: true
           R_LINTR_LINTER_FILE: .ci/.lintr
+  lint-c:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+      - uses: r-lib/actions/setup-r@v2
+      - name: Lint
+        run: |
+          for (f in list.files('.ci/linters', full.names=TRUE)) source(f)
+          for (f in list.files('src', pattern='[.]c$', full.names=TRUE)) {
+            alloc_linter(f)
+          }

From 40cb39a103fb4331d122558d56eb9ebcbf0fb8b4 Mon Sep 17 00:00:00 2001
From: Michael Chirico <chiricom@google.com>
Date: Tue, 16 Jul 2024 06:09:28 +0000
Subject: [PATCH 11/15] TODO for later

---
 .github/workflows/code-quality.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/.github/workflows/code-quality.yaml b/.github/workflows/code-quality.yaml
index 20222b606c..611e828275 100644
--- a/.github/workflows/code-quality.yaml
+++ b/.github/workflows/code-quality.yaml
@@ -43,4 +43,5 @@ jobs:
           for (f in list.files('.ci/linters', full.names=TRUE)) source(f)
           for (f in list.files('src', pattern='[.]c$', full.names=TRUE)) {
             alloc_linter(f)
+            # TODO(#6272): Incorporate more checks from CRAN_Release
           }

From bdfa6f0a4d0fe5506d7cc96bbeaf542e2d1f2c1a Mon Sep 17 00:00:00 2001
From: Michael Chirico <chiricom@google.com>
Date: Mon, 15 Jul 2024 23:11:38 -0700
Subject: [PATCH 12/15] Need to specify run -- as R

---
 .github/workflows/code-quality.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/.github/workflows/code-quality.yaml b/.github/workflows/code-quality.yaml
index 611e828275..1a8376c55e 100644
--- a/.github/workflows/code-quality.yaml
+++ b/.github/workflows/code-quality.yaml
@@ -45,3 +45,4 @@ jobs:
             alloc_linter(f)
             # TODO(#6272): Incorporate more checks from CRAN_Release
           }
+        shell: Rscript {0}

From c94d42d15cd283aaf9a0a706f63d53a080eaa3a5 Mon Sep 17 00:00:00 2001
From: Michael Chirico <chiricom@google.com>
Date: Tue, 16 Jul 2024 06:21:28 +0000
Subject: [PATCH 13/15] move linters to own subdirectories

---
 .ci/linters/{ => c}/alloc_linter.R           | 0
 .ci/linters/{ => r}/dt_lengths_linter.R      | 0
 .ci/linters/{ => r}/dt_test_literal_linter.R | 0
 3 files changed, 0 insertions(+), 0 deletions(-)
 rename .ci/linters/{ => c}/alloc_linter.R (100%)
 rename .ci/linters/{ => r}/dt_lengths_linter.R (100%)
 rename .ci/linters/{ => r}/dt_test_literal_linter.R (100%)

diff --git a/.ci/linters/alloc_linter.R b/.ci/linters/c/alloc_linter.R
similarity index 100%
rename from .ci/linters/alloc_linter.R
rename to .ci/linters/c/alloc_linter.R
diff --git a/.ci/linters/dt_lengths_linter.R b/.ci/linters/r/dt_lengths_linter.R
similarity index 100%
rename from .ci/linters/dt_lengths_linter.R
rename to .ci/linters/r/dt_lengths_linter.R
diff --git a/.ci/linters/dt_test_literal_linter.R b/.ci/linters/r/dt_test_literal_linter.R
similarity index 100%
rename from .ci/linters/dt_test_literal_linter.R
rename to .ci/linters/r/dt_test_literal_linter.R

From 1cb8e1a10bb4622f2f802cf82af203f16cc13df7 Mon Sep 17 00:00:00 2001
From: Michael Chirico <chiricom@google.com>
Date: Tue, 16 Jul 2024 06:22:15 +0000
Subject: [PATCH 14/15] reflect new directories where needed

---
 .ci/.lintr.R                        | 2 +-
 .github/workflows/code-quality.yaml | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/.ci/.lintr.R b/.ci/.lintr.R
index 6659efbc82..2481ecec97 100644
--- a/.ci/.lintr.R
+++ b/.ci/.lintr.R
@@ -1,5 +1,5 @@
 dt_linters = new.env()
-for (f in list.files('.ci/linters', full.names=TRUE)) sys.source(f, dt_linters)
+for (f in list.files('.ci/linters/r', full.names=TRUE)) sys.source(f, dt_linters)
 rm(f)
 
 # NB: Could do this inside the linter definition, this separation makes those files more standardized
diff --git a/.github/workflows/code-quality.yaml b/.github/workflows/code-quality.yaml
index 1a8376c55e..f9066eacd0 100644
--- a/.github/workflows/code-quality.yaml
+++ b/.github/workflows/code-quality.yaml
@@ -40,7 +40,7 @@ jobs:
       - uses: r-lib/actions/setup-r@v2
       - name: Lint
         run: |
-          for (f in list.files('.ci/linters', full.names=TRUE)) source(f)
+          for (f in list.files('.ci/linters/c', full.names=TRUE)) source(f)
           for (f in list.files('src', pattern='[.]c$', full.names=TRUE)) {
             alloc_linter(f)
             # TODO(#6272): Incorporate more checks from CRAN_Release

From a4aea85dd1d12fd51d5ff20ef1ff31ea82f57111 Mon Sep 17 00:00:00 2001
From: Michael Chirico <chiricom@google.com>
Date: Tue, 16 Jul 2024 07:14:11 -0700
Subject: [PATCH 15/15] NEWS

---
 NEWS.md | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/NEWS.md b/NEWS.md
index 262aace175..ab412e0524 100644
--- a/NEWS.md
+++ b/NEWS.md
@@ -116,6 +116,8 @@
 
 21. Refactored some non-API calls to R macros for S4 objects (#6180)[https://github.com/Rdatatable/data.table/issues/6180]. There should be no user-visible change. Thanks to various R users & R core for pushing to have a clearer definition of "API" for R, and thanks @MichaelChirico for implementing here.
 
+22. C code was unified more in how failures to allocate memory (`malloc()`/`calloc()`) are handled, (#1115)[https://github.com/Rdatatable/data.table/issues/1115]. No OOM issues were reported, as these regions of code typically request relatively small blocks of memory, but it is good to handle memory pressure consistently. Thanks @elfring for the report and @MichaelChirico for the clean-up effort and future-proofing linter.
+
 ## TRANSLATIONS
 
 1. Fix a typo in a Mandarin translation of an error message that was hiding the actual error message, [#6172](https://github.com/Rdatatable/data.table/issues/6172). Thanks @trafficfan for the report and @MichaelChirico for the fix.