[gnuastro-commits] master 1624f4e9: Library (txt.h): last columns with string format correctly read

[Mohammad Akhlaghi]

branch: master
commit 1624f4e9aec26d6373f315dd99d71623eed42ead
Author: Mohammad Akhlaghi <mohammad@akhlaghi.org>
Commit: Mohammad Akhlaghi <mohammad@akhlaghi.org>

    Library (txt.h): last columns with string format correctly read
    
    Until now, when the last column of a table was a string, and the specified
    width (in the metadata) is larger than the width of the string (before the
    new-line character), Table would print unreasonable characters after the
    main column value. For example see this command
    
        printf "# Column 1: RA [s, str20]\n%s" 03:24:15 | asttable
    
    In the command above, the string metadata has given a width of 20
    characters, while the actual string is 8 characters! So some extra
    non-sense characters are printed after the output! If we set the metadata
    to 'str8' or 'str9' there is no problem (the 'str9' will also copy the
    string termination character).
    
    With this commit, the cause of the problem has been found and fixed. The
    main problem was that we were only copying the shorter content, but we
    weren't setting the string termination character in the end of the copied
    number of bytes.
    
    I also noticed that for allocating new space, we were directly using the
    low-level 'malloc' instead of the high-level 'gal_pointer_allocate' which
    is also cleaner. We now use the high-level allocation function.
    
    This bug was found based on a discussion with Manuel Sánchez-Benavente.
    
    This fixes bug #62720.
---
 NEWS      |  2 ++
 lib/txt.c | 26 ++++++++++++++++----------
 2 files changed, 18 insertions(+), 10 deletions(-)

diff --git a/NEWS b/NEWS
index 9060070d..7261b6ce 100644
--- a/NEWS
+++ b/NEWS
@@ -218,6 +218,8 @@ See the end of the file for license conditions.
               data analysis). Found by Manuel Sánchez-Benavente.
   bug #62718: Table goes into an infinite loop on some old ASCII FITS
               tables. Found by Hilderic Browne.
+  bug #62720: Table not reading string columns that are shorter than the
+              defined width in the metadata.
 
 
 
diff --git a/lib/txt.c b/lib/txt.c
index c8ac34c6..f102d1eb 100644
--- a/lib/txt.c
+++ b/lib/txt.c
@@ -36,6 +36,7 @@ along with Gnuastro. If not, see 
<http://www.gnu.org/licenses/>.
 #include <gnuastro/units.h>
 #include <gnuastro/blank.h>
 #include <gnuastro/table.h>
+#include <gnuastro/pointer.h>
 
 #include <gnuastro-internal/checkset.h>
 #include <gnuastro-internal/tableintern.h>
@@ -936,7 +937,7 @@ txt_fill(char *in_line, char **tokens, size_t maxcolnum,
 {
   gal_data_t *data;
   int notenoughcols=0;
-  size_t i, n=0, strwidth;
+  size_t i, len, n=0, strwidth;
   char *end, *line, *tmpstr, *aline=NULL;
 
   /* Make a copy of the input line if necessary. */
@@ -979,16 +980,21 @@ txt_fill(char *in_line, char **tokens, size_t maxcolnum,
              copy the necessary number of characters into the 'tmpstr'
              string. We need to allocate this because the string column may
              be immediately (next character) followed by the next
-             column. This leaves us no space to put the '\0' character. */
+             column. This leaves us no space to copy the '\0'
+             character. Therefore we will add '\0' after 'strncpy'.
+
+             Also, it may happen (for example with plain-text editors that
+             remove trailing white space) that the width defined for the
+             last column becomes larger than the actual length of the
+             line. We should therefore first check how many characters we
+             should actually copy (may be less than 'disp_width'). See
+             https://savannah.gnu.org/bugs/index.php?62720 */
           strwidth=colinfo[n-1].disp_width;
-          errno=0;
-          tmpstr=malloc(strwidth+1);
-          if(tmpstr==NULL)
-            error(EXIT_FAILURE, errno, "%s: %zu bytes couldn't be allocated "
-                  "for variable 'tmpstr'", __func__, strwidth+1);
-          if(line+strwidth<end) strncpy(tmpstr, line, strwidth);
-          else                  strncpy(tmpstr, line, end-line);
-          tmpstr[strwidth]='\0';
+          len = (line+strwidth)<end ? strwidth : end-line;
+          tmpstr=gal_pointer_allocate(GAL_TYPE_UINT8, len+1, 0,
+                                      __func__, "tmpstr");
+          strncpy(tmpstr, line, len);
+          tmpstr[len]='\0';
           tokens[n]=tmpstr;
 
           /* Increment the line pointer beyond to the next token.*/