Merge pull request #472 from ycphs/fix_rdevel_segfaults

Fix rdevel segfaults
ycphs · Jun 3, 2024 · e49cfc5 · e49cfc5
2 parents 67b75d4 + f3d212e
commit e49cfc5
Show file tree

Hide file tree

Showing 19 changed files with 82 additions and 52 deletions.
diff --git a/.github/workflows/R-CMD-check.yaml b/.github/workflows/R-CMD-check.yaml
@@ -1,15 +1,15 @@
 # Workflow derived from https://github.com/r-lib/actions/tree/v2/examples
+# Need help debugging build failures? Start at https://github.com/r-lib/actions#where-to-find-help
 on:
   push:
     branches: [main, master]
   pull_request:
     branches: [main, master]
-  schedule:
-    # * is a special character in YAML so you have to quote this string
-    - cron:  '0 12 * * *'
 
 name: R-CMD-check
 
+permissions: read-all
+
 jobs:
   R-CMD-check:
     runs-on: ${{ matrix.config.os }}
@@ -20,25 +20,18 @@ jobs:
       fail-fast: false
       matrix:
         config:
-          - {os: windows-latest, r: 'oldrel-1'}
+          - {os: macos-latest,   r: 'release'}
           - {os: windows-latest, r: 'release'}
-          - {os: windows-latest, r: 'devel', http-user-agent: 'release'}
-          - {os: macOS-latest, r: 'oldrel-1'}
-          - {os: macOS-latest, r: 'release'}
-          - {os: macOS-latest, r: 'devel', http-user-agent: 'release'}
-          - {os: ubuntu-latest, r: 'oldrel-1'}
-          - {os: ubuntu-latest, r: 'release'}
-          - {os: ubuntu-latest, r: 'devel', http-user-agent: 'release'}
+          - {os: ubuntu-latest,   r: 'devel', http-user-agent: 'release'}
+          - {os: ubuntu-latest,   r: 'release'}
+          - {os: ubuntu-latest,   r: 'oldrel-1'}
 
     env:
-      R_REMOTES_NO_ERRORS_FROM_WARNINGS: true
-      RSPM: ${{ matrix.config.rspm }}
       GITHUB_PAT: ${{ secrets.GITHUB_TOKEN }}
-
-    if: "!contains(github.event.head_commit.message, 'revdepcheck run - No changes to commit')"
+      R_KEEP_PKG_SOURCE: yes
 
     steps:
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@v4
 
       - uses: r-lib/actions/setup-pandoc@v2
 
@@ -55,4 +48,5 @@ jobs:
 
       - uses: r-lib/actions/check-r-package@v2
         with:
-          upload-snapshots: true
+          upload-snapshots: true
+          build_args: 'c("--no-manual","--compact-vignettes=gs+qpdf")'
diff --git a/.gitignore b/.gitignore
@@ -19,3 +19,4 @@ docs/
 openxlsx.Rcheck/
 openxlsx*.tar.gz
 openxlsx*.tgz
+vignettes/*
diff --git a/DESCRIPTION b/DESCRIPTION
@@ -60,7 +60,7 @@ VignetteBuilder:
     knitr
 Encoding: UTF-8
 Language: en-US
-RoxygenNote: 7.2.0
+RoxygenNote: 7.3.1
 Collate: 
     'CommentClass.R'
     'HyperlinkClass.R'
@@ -95,3 +95,4 @@ Collate:
     'writexlsx.R'
     'zzz.R'
 Roxygen: list(markdown = TRUE)
+LazyData: true
diff --git a/R/conditional_formatting.R b/R/conditional_formatting.R
@@ -376,7 +376,7 @@ conditionalFormatting <-
         stop("If type == 'colourScale', style must be a vector of colours of length 2 or 3.")
       }
 
-      if (class(style) != "character") {
+      if (!inherits(style, "character")) {
         stop("If type == 'colourScale', style must be a vector of colours of length 2 or 3.")
       }
 
@@ -404,7 +404,7 @@ conditionalFormatting <-
         style <- "#638EC6"
       }
 
-      if (class(style) != "character") {
+      if (!inherits(style, "character")) {
         stop("If type == 'dataBar', style must be a vector of colours of length 1 or 2.")
       }
 

diff --git a/R/data-fontSizeLookupTables.R b/R/data-fontSizeLookupTables.R
@@ -1,8 +1,11 @@
 #' Font Size Lookup tables
-#' 
-#' Lookup tables for font size 
-#' 
+#'
+#' Lookup tables for font size
+#'
 #' @format A data.frame with column names corresponding to font names
+#' @examples
+#' data(openxlsxFontSizeLookupTable)
+#' data(openxlsxFontSizeLookupTableBold)
 "openxlsxFontSizeLookupTable"
 
 #' @rdname openxlsxFontSizeLookupTable

diff --git a/R/openxlsx.R b/R/openxlsx.R
@@ -53,7 +53,7 @@
 #' Additional options
 #'
 #'
- NULL
+ "_PACKAGE"
 
 #' openxlsx Options
 #' 

diff --git a/R/openxlsxCoerce.R b/R/openxlsxCoerce.R
@@ -2,16 +2,18 @@
 
 ##
 
+#' @noRd
 openxlsxCoerce <- function(x, rowNames) {
   UseMethod("openxlsxCoerce")
 }
 
+#' @noRd
 openxlsxCoerce.default <- function(x, rowNames) {
   x <- as.data.frame(x, stringsAsFactors = FALSE)
   return(x)
 }
 
-
+#' @noRd
 openxlsxCoerce.data.frame <- function(x, rowNames) {
 
   ## cbind rownames to x
@@ -23,7 +25,7 @@ openxlsxCoerce.data.frame <- function(x, rowNames) {
   return(x)
 }
 
-
+#' @noRd
 openxlsxCoerce.data.table <- function(x, rowNames) {
   x <- as.data.frame(x, stringsAsFactors = FALSE)
 
@@ -36,7 +38,7 @@ openxlsxCoerce.data.table <- function(x, rowNames) {
   return(x)
 }
 
-
+#' @noRd
 openxlsxCoerce.matrix <- function(x, rowNames) {
   x <- as.data.frame(x, stringsAsFactors = FALSE)
 
@@ -48,11 +50,12 @@ openxlsxCoerce.matrix <- function(x, rowNames) {
   return(x)
 }
 
-
+#' @noRd
 openxlsxCoerce.array <- function(x, rowNames) {
   stop("array in writeData : currently not supported")
 }
 
+#' @noRd
 openxlsxCoerce.aov <- function(x, rowNames) {
   x <- summary(x)
   x <- cbind(x[[1]])
@@ -62,7 +65,7 @@ openxlsxCoerce.aov <- function(x, rowNames) {
   return(x)
 }
 
-
+#' @noRd
 openxlsxCoerce.lm <- function(x, rowNames) {
   x <- as.data.frame(summary(x)[["coefficients"]])
   x <- cbind(data.frame("Variable" = rownames(x), stringsAsFactors = FALSE), x)
@@ -71,7 +74,7 @@ openxlsxCoerce.lm <- function(x, rowNames) {
   return(x)
 }
 
-
+#' @noRd
 openxlsxCoerce.anova <- function(x, rowNames) {
   x <- as.data.frame(x)
 
@@ -83,7 +86,7 @@ openxlsxCoerce.anova <- function(x, rowNames) {
   return(x)
 }
 
-
+#' @noRd
 openxlsxCoerce.glm <- function(x, rowNames) {
   x <- as.data.frame(summary(x)[["coefficients"]])
   x <- cbind(data.frame("row name" = rownames(x), stringsAsFactors = FALSE), x)
@@ -92,7 +95,7 @@ openxlsxCoerce.glm <- function(x, rowNames) {
   return(x)
 }
 
-
+#' @noRd
 openxlsxCoerce.table <- function(x, rowNames) {
   x <- as.data.frame(unclass(x))
   x <- cbind(data.frame("Variable" = rownames(x), stringsAsFactors = FALSE), x)
@@ -101,7 +104,7 @@ openxlsxCoerce.table <- function(x, rowNames) {
   return(x)
 }
 
-
+#' @noRd
 openxlsxCoerce.prcomp <- function(x, rowNames) {
   x <- as.data.frame(x$rotation)
   x <- cbind(data.frame("Variable" = rownames(x), stringsAsFactors = FALSE), x)
@@ -110,7 +113,7 @@ openxlsxCoerce.prcomp <- function(x, rowNames) {
   return(x)
 }
 
-
+#' @noRd
 openxlsxCoerce.summary.prcomp <- function(x, rowNames) {
   x <- as.data.frame(x$importance)
   x <- cbind(data.frame("Variable" = rownames(x), stringsAsFactors = FALSE), x)
@@ -171,7 +174,7 @@ openxlsxCoerce.survdiff <- function(x, rowNames) {
 }
 
 
-
+#' @noRd
 openxlsxCoerce.coxph <- function(x, rowNames) {
 
   ## sligthly modified print.coxph
@@ -199,7 +202,7 @@ openxlsxCoerce.coxph <- function(x, rowNames) {
 
 
 
-
+#' @noRd
 openxlsxCoerce.summary.coxph <- function(x, rowNames) {
   coef <- x$coefficients
   ci <- x$conf.int
@@ -219,6 +222,7 @@ openxlsxCoerce.summary.coxph <- function(x, rowNames) {
   return(x)
 }
 
+#' @noRd
 openxlsxCoerce.cox.zph <- function(x, rowNames) {
   tmp <- as.data.frame(x$table)
   x <- cbind(data.frame("row names" = rownames(tmp)), tmp)
@@ -227,7 +231,7 @@ openxlsxCoerce.cox.zph <- function(x, rowNames) {
   return(x)
 }
 
-
+#' @noRd
 openxlsxCoerce.hyperlink <- function(x, rowNames) {
   ## vector of hyperlinks
   class(x) <- c("character", "hyperlink")

diff --git a/R/sysdata.rda b/R/sysdata.rda
diff --git a/R/wrappers.R b/R/wrappers.R
@@ -1754,7 +1754,7 @@ getStyles <- function(wb) {
 #' saveWorkbook(wb, "removeWorksheetExample.xlsx", overwrite = TRUE)
 #' }
 removeWorksheet <- function(wb, sheet) {
-  if (class(wb) != "Workbook") {
+  if (!inherits(wb, "Workbook")) {
     stop("wb must be a Workbook object!")
   }
 

diff --git a/data/openxlsxFontSizeLookupTable.rda b/data/openxlsxFontSizeLookupTable.rda
diff --git a/data/openxlsxFontSizeLookupTableBold.rda b/data/openxlsxFontSizeLookupTableBold.rda
diff --git a/man/deleteDataColumn.Rd b/man/deleteDataColumn.Rd
diff --git a/man/openxlsx.Rd b/man/openxlsx.Rd
diff --git a/man/openxlsxFontSizeLookupTable.Rd b/man/openxlsxFontSizeLookupTable.Rd
diff --git a/src/load_workbook.cpp b/src/load_workbook.cpp
@@ -119,7 +119,7 @@ SEXP loadworksheets(Reference wb, List styleObjects, std::vector<std::string> xm
 
           buf = cols[ci];
           // If either custom widths or groupings, get column index
-          if ((buf.find("customWidth", 0) != string::npos) | (buf.find("outlineLevel", 0) != string::npos)) {
+          if ((buf.find("customWidth", 0) != string::npos) || (buf.find("outlineLevel", 0) != string::npos)) {
 
             tmp_pos = buf.find("min=\"", 0);
             endPos = buf.find(tagEnd, tmp_pos + 5);
@@ -139,7 +139,7 @@ SEXP loadworksheets(Reference wb, List styleObjects, std::vector<std::string> xm
             }
 
             // If column has both a custom width and is part of a group
-            if ((buf.find("customWidth", 0) != string::npos) & (buf.find("outlineLevel", 0) != string::npos)) {
+            if ((buf.find("customWidth", 0) != string::npos) && (buf.find("outlineLevel", 0) != string::npos)) {
               tmp_pos = buf.find("width=\"", 0);
               endPos = buf.find(tagEnd, tmp_pos + 7);
               tmp_width = atof(buf.substr(tmp_pos + 7, endPos - tmp_pos - 7).c_str()) - 0.71;

diff --git a/src/read_workbook.cpp b/src/read_workbook.cpp
@@ -500,7 +500,10 @@ SEXP read_workbook(IntegerVector cols_in,
 
   bool has_strings = true;
   IntegerVector st_inds0 (1);
-  st_inds0[0] = string_inds[0];
+
+  if (string_inds.size()) 
+    st_inds0[0] = string_inds[0];
+
   if(is_true(all(is_na(st_inds0))))
     has_strings = false;
 
@@ -549,8 +552,7 @@ SEXP read_workbook(IntegerVector cols_in,
     for(unsigned short i=0; i < nCols; i++){
 
       if(missing_header[i]){  // a missing header element
-
-        sprintf(&(name[0]), "X%hu", i+1);
+        snprintf(&(name[0]), sizeof(name), "X%hu", i+1);
         // sprintf(&(name[0]), "X%u", i+1);
         // snprintf(&(name[0]), sizeof(&(name[0])), "X%d", i+1);
         // snprintf(&(name[0]), 10, "X%d", i+1);
@@ -560,7 +562,8 @@ SEXP read_workbook(IntegerVector cols_in,
 
         col_names[i] = v[pos];
         if(col_names[i] == "NA"){
-          sprintf(&(name[0]), "X%hu", i+1);
+          snprintf(&(name[0]), sizeof(name), "X%hu", i+1);
+
           // sprintf(&(name[0]), "X%du", i+1);
           // snprintf(&(name[0]), sizeof(&(name[0])), "X%d", i+1);
           // snprintf(&(name[0]), 10, "X%d", i+1);
@@ -620,7 +623,8 @@ SEXP read_workbook(IntegerVector cols_in,
   }else{ // else col_names is FALSE
     char name[6];
     for(int i =0; i < nCols; i++){
-      sprintf(&(name[0]), "X%hu", i+1);
+      snprintf(&(name[0]), sizeof(name), "X%hu", i+1);
+
        // snprintf(&(name[0]), sizeof(&(name[0])), "X%d", i+1);
       // sprintf(&(name[0]), "X%u", i+1);
       col_names[i] = name;
@@ -637,7 +641,7 @@ SEXP read_workbook(IntegerVector cols_in,
   // Possible there are no string_inds to begin with and value of string_inds is 0
   // Possible we have string_inds but they have now all been used up by headers
   bool allNumeric = false;
-  if((string_inds.size() == 0) | all(is_na(string_inds)))
+  if((string_inds.size() == 0) || is_true(all(is_na(string_inds))))
     allNumeric = true;
 
   if(has_date){