Avoid memcpy() of empty vector (#6911)

* Avoid memcpy() of empty vector

* in getidcols() too

* Add a test case

Distilled from the `melt()` call here:

https://github.com/gdemin/expss/blob/d498caf72a9938ac71b9e06fd61485cd0347d607/tests/testthat/test_subtotal.R#L161-L166

Author: MichaelChirico

inst/tests/tests.Rraw

@@ -21129,3 +21129,8 @@ test(2311.2, nlevels(DT$V1), 2L) # used to be 3
 # avoid translateChar*() in OpenMP threads, #6883
 DF = list(rep(iconv("\uf8", from = "UTF-8", to = "latin1"), 2e6))
 test(2312, fwrite(DF, nullfile(), encoding = "UTF-8", nThread = 2L), NULL)
+
+# avoid memcpy of 0-length inputs
+test(2313,
+     melt(data.table(a=numeric(), b=numeric(), c=numeric()), id.vars=c('a', 'b')),
+     data.table(a=numeric(), b=numeric(), variable=factor(levels='c'), value=numeric()))

src/fmelt.c

@@ -552,23 +552,23 @@ SEXP getvaluecols(SEXP DT, SEXP dtnames, Rboolean valfactor, Rboolean verbose, s
           //TODO complex value type: case CPLXSXP: { } break;
         case REALSXP : {
           double *dtarget = REAL(target);
-          const double *dthiscol = REAL(thiscol);
+          const double *dthiscol = REAL_RO(thiscol);
           if (data->narm) {
             for (int k=0; k<thislen; ++k)
               dtarget[counter + k] = dthiscol[ithisidx[k]-1];
-          } else {
+          } else if (data->nrow) {
             memcpy(dtarget + j*data->nrow, dthiscol, data->nrow*size);
           }
         }
           break;
         case INTSXP :
         case LGLSXP : {
           int *itarget = INTEGER(target);
-          const int *ithiscol = INTEGER(thiscol);
+          const int *ithiscol = INTEGER_RO(thiscol);
           if (data->narm) {
             for (int k=0; k<thislen; ++k)
               itarget[counter + k] = ithiscol[ithisidx[k]-1];
-          } else {
+          } else if (data->nrow) {
             memcpy(itarget + j*data->nrow, ithiscol, data->nrow*size);
           }
         } break;
@@ -704,7 +704,7 @@ SEXP getidcols(SEXP DT, SEXP dtnames, Rboolean verbose, struct processData *data
     switch(TYPEOF(thiscol)) {
     case REALSXP : {
       double *dtarget = REAL(target);
-      const double *dthiscol = REAL(thiscol);
+      const double *dthiscol = REAL_RO(thiscol);
       if (data->narm) {
         for (int j=0; j<data->lmax; ++j) {
           SEXP thisidx = VECTOR_ELT(data->not_NA_indices, j);
@@ -714,7 +714,7 @@ SEXP getidcols(SEXP DT, SEXP dtnames, Rboolean verbose, struct processData *data
             dtarget[counter + k] = dthiscol[ithisidx[k]-1];
           counter += thislen;
         }
-      } else {
+      } else if (data->nrow) {
         for (int j=0; j<data->lmax; ++j)
           memcpy(dtarget + j*data->nrow, dthiscol, data->nrow*size);
       }
@@ -723,7 +723,7 @@ SEXP getidcols(SEXP DT, SEXP dtnames, Rboolean verbose, struct processData *data
     case INTSXP :
     case LGLSXP : {
       int *itarget = INTEGER(target);
-      const int *ithiscol = INTEGER(thiscol);
+      const int *ithiscol = INTEGER_RO(thiscol);
       if (data->narm) {
         for (int j=0; j<data->lmax; ++j) {
           SEXP thisidx = VECTOR_ELT(data->not_NA_indices, j);
@@ -733,7 +733,7 @@ SEXP getidcols(SEXP DT, SEXP dtnames, Rboolean verbose, struct processData *data
             itarget[counter + k] = ithiscol[ithisidx[k]-1];
           counter += thislen;
         }
-      } else {
+      } else if (data->nrow) {
         for (int j=0; j<data->lmax; ++j)
           memcpy(itarget + j*data->nrow, ithiscol, data->nrow*size);
       }