(ACS) Implement variadic trailer functions (#644)

* Variadic trl messaging functions

* Increase message buffer length to 2Kb

* Use snprintf

* Fix allocation of obsnames array

* Add extra byte for nul terminator

* Fix ODR violation

* status is already global

* Report the (potentially bad) format string

* Fix overflow in ioerr

* Formatting

* Replace magic number usage

* Remove VLA

* Temporarily escape scale noise format

* The new trlmessage interprets '%%' as an invalid formatter

* Fix ODR violation

* MsgText is already global

* status is already global

* ACS: Use variadic trailer functions

* Replaces sprintf+trl(message|warn|error) with single trl calls

* Redundant message handlers have been purged

* Fix snprintf size mismatch

* Remove errant line feeds

* Fix error message

* Say calloc not malloc

* Fix bad printf call

* Use HSTCAL-specific error codes on exit

* Update lib/hstcalversion.c

MsgText is written to the header/history

* Remove errant semi-colons

* Fix MSG_BUFF_LENGTH definition

* Fix missing warning

* Fix incorrect message type

* Remove unused function prototypes

* acs.h: asnerror
* acs.h: asnwarn

Co-authored-by: Michele De La Pena <[email protected]>

---------

Co-authored-by: P. L. Lim <[email protected]>
Co-authored-by: Michele De La Pena <[email protected]>

Author: 3 people

ctegen2/ctehelpers.c

@@ -252,7 +252,7 @@ No.    Name      Ver    Type      Cards   Dimensions   Format
     extern int status; /* variable for return status */
 
     /* VARIABLE FOR FILENAME + EXTENSION NUMBER. */
-    char filename_wext[strlen(filename) + 4];
+    char filename_wext[CHAR_FNAME_LENGTH] = {0};
 
     /* NAMES OF DATA COLUMNS WE WANT FROM THE FILE, DATA WILL BE STORED IN THE PARS STRUCTURE */
     /* QPROF table - the last column is a description string */

hstio/hstio.c

@@ -427,11 +427,12 @@ static void ioerr(HSTIOError e, IODescPtr x_, int status) {
         IODesc *x;
         char cfitsio_errmsg[81];
         x = (IODesc *)x_;
-        sprintf(&error_msg[strlen(error_msg)],
+        snprintf(&error_msg[strlen(error_msg)],
+                sizeof(error_msg) - strlen(error_msg),
                 "Filename %s EXTNAME %s EXTVER %d CFITSIO status %d\n",
                 x->filename, x->extname, x->extver, status);
         while (fits_read_errmsg(cfitsio_errmsg)) {
-            strncat(error_msg, cfitsio_errmsg, 80);
+            strncat(error_msg, cfitsio_errmsg, sizeof(error_msg) - strlen(error_msg) - 1);
         }
         error(e,0);
 }
@@ -2050,8 +2051,7 @@ static char *make_iodesc(IODesc **x, char *fname, char *ename, int ever) {
         iodesc->type = 0;
         if (fname == 0) fname = "";
         if (ename == 0) ename = "";
-        iodesc->filename = (char *)calloc(((flen = strlen(fname)) + 1),
-                sizeof(char));
+        iodesc->filename = (char *)calloc(((flen = strlen(fname)) + 1), sizeof(char));
         if (iodesc->filename == NULL) {
             free(iodesc);
             error(NOMEM,"Allocating I/O descriptor");
@@ -2077,13 +2077,14 @@ static char *make_iodesc(IODesc **x, char *fname, char *ename, int ever) {
 
         /* make up the proper filename */
         /* check for a request for the primary HDU */
-        tmp = (char *)calloc((flen + 80),sizeof(char));
+        const size_t flen_new = flen + 80;
+        tmp = (char *)calloc(flen_new,sizeof(char));
         if (tmp == NULL) { error(NOMEM,"Allocating I/O descriptor"); return NULL; }
         strcpy(tmp,fname);
         if (ever == 0 || ename == 0 || ename[0] == '\0' || ename[0] == ' ')
             strcat(tmp,"[0]");
         else
-            sprintf(&tmp[flen],"[%s,%d]",xname,ever);
+            snprintf(&tmp[flen], flen_new, "[%s,%d]",xname,ever);
 
         *x = iodesc;
         return tmp;

hstio/keyword.c

@@ -1407,9 +1407,9 @@ char *comment     i: comment to add, if keyword doesn't exist
         char strBuffer[MSG_BUFF_LENGTH];
         *strBuffer = '\0';
         if (value == True)
-            sprintf(strBuffer, "%s T %s", keyword, comment);
+            snprintf(strBuffer, sizeof(strBuffer), "%s T %s", keyword, comment);
         else if(value == False)
-            sprintf(strBuffer, "%s F %s", keyword, comment);
+            snprintf(strBuffer, sizeof(strBuffer), "%s F %s", keyword, comment);
         else
             assert(0); // Unimplemented
         tempStatus = addHistoryKw(hd, strBuffer);
@@ -1487,7 +1487,7 @@ char *comment     i: comment to add, if keyword doesn't exist
         // add as history if keyword missing
         char strBuffer[MSG_BUFF_LENGTH];
         *strBuffer = '\0';
-        sprintf(strBuffer, "%s %s %s", keyword, value, comment);
+        snprintf(strBuffer, sizeof(strBuffer), "%s %s %s", keyword, value, comment);
         tempStatus = addHistoryKw(hd, strBuffer);
     }
     return tempStatus;

include/hstcal.h

@@ -8,8 +8,8 @@
 //
 // Note: IRAF_SZ_LINE = 1023 relates to the maximum filename size accepted by cfitsio.
 #define CHAR_FNAME_LENGTH 255
-#define CHAR_LINE_LENGTH 255
-#define MSG_BUFF_LENGTH CHAR_LINE_LENGTH + 1
+#define CHAR_LINE_LENGTH 1023
+#define MSG_BUFF_LENGTH ((CHAR_LINE_LENGTH * 2) + 1)
 
 /* Standard string buffer for use in messages */
 extern char MsgText[MSG_BUFF_LENGTH];

include/trlbuf.h

@@ -28,9 +28,9 @@ void SetTrlQuietMode (int quiet);
 void InitTrlPreface (void);
 void ResetTrlPreface (void);
 void CloseTrlBuf (struct TrlBuf * ptr);
-void trlmessage (const char *message);
-void trlwarn (const char *message);
-void trlerror (const char *message);
+void trlmessage (const char *fmt, ...);
+void trlwarn (const char *fmt, ...);
+void trlerror (const char *fmt, ...);
 void trlopenerr (const char *filename);
 void trlreaderr (const char *filename);
 void trlkwerr (const char *keyword, const char *filename);

lib/getphttab.c

@@ -312,14 +312,14 @@ static int OpenPhotTab (char *tabname, char *photvar, PhtCols *tabinfo) {
     /* Copy in the column names now */
     strcpy(colnames[0],photvar);
     for (parnum=1;parnum <= tabinfo->parnum; parnum++){
-        sprintf(colnames[parnum],"%s%i",photvar,parnum);
-        sprintf(ecolnames[parnum],"NELEM%i",parnum);
-        sprintf(pncolnames[parnum],"PAR%iNAMES",parnum);
-        sprintf(pvcolnames[parnum],"PAR%iVALUES",parnum);
+        snprintf(colnames[parnum], SZ_COLNAME, "%s%i", photvar, parnum);
+        snprintf(ecolnames[parnum], SZ_COLNAME, "NELEM%i", parnum);
+        snprintf(pncolnames[parnum], SZ_COLNAME, "PAR%iNAMES", parnum);
+        snprintf(pvcolnames[parnum], SZ_COLNAME, "PAR%iVALUES", parnum);
     }
 
     /* Create name of table with extension to be opened */
-    sprintf(tname,"%s[%s]",tabname,photvar);
+    snprintf(tname, sizeof(tname), "%s[%s]", tabname, photvar);
 
     /* keep track of what extension we are processing here */
     strcpy(tabinfo->photvar, photvar);
@@ -497,9 +497,9 @@ static int InterpretPhotmode(char *photmode, PhotPar *obs){
         /*
            Initialize intermediate variable
          */
-        obsnames = (char **)calloc(obselems,sizeof(char *));
+        obsnames = calloc(obselems + 1,sizeof(*obsnames));
         for (i=0;i<obselems;i++)
-            obsnames[i] = (char *) calloc(SZ_COLNAME,sizeof(char));
+            obsnames[i] = (char *) calloc(SZ_COLNAME + 1,sizeof(char));
         /* Start adding elements to the intermediate variable */
         strcpy(tempmode,photmode);
         chtok=strtok(tempmode,",");
@@ -695,8 +695,8 @@ static int ReadPhotArray (PhtCols *tabinfo, int row, PhtRow *tabrow) {
         for (n=1,col=0; n<=tabrow->parnum; n++,col++){
             /* Build up names of columns for parameterized values
                which need to be read in */
-            sprintf(col_nelem,"NELEM%d",n);    /* col name: NELEMn */
-            sprintf(col_parval,"PAR%dVALUES",n); /* col name: PARnVALUES */
+            snprintf(col_nelem, sizeof(col_nelem), "NELEM%d",n);    /* col name: NELEMn */
+            snprintf(col_parval, sizeof(col_parval), "PAR%dVALUES",n); /* col name: PARnVALUES */
 
             /* Read in number of elements for this parameterized variable */
             c_tbegti (tabinfo->tp, tabinfo->cp_nelem[n], row, &tabrow->nelem[n]);

lib/hstcalversion.c

@@ -14,10 +14,10 @@ char * sprintfGitInfo(char ** buffer)
 
     const char * format = "git tag: %s\ngit branch: %s\nHEAD @: %s";
     size_t length = strlen(format) + strlen(VERSION) + strlen(BRANCH) + strlen(COMMIT); // NOTE: this doesn't require an explicit +1 for '\0' as it is larger than needed due to '%s' & '\n'.
-    *buffer = malloc(length*sizeof(char));
+    *buffer = malloc((length + 1) * sizeof(char));
     if (!*buffer)
         return NULL;
-    sprintf(*buffer, format, VERSION, BRANCH, COMMIT);
+    snprintf(*buffer, length, format, VERSION, BRANCH, COMMIT);
     return *buffer;
 }