[gnuastro-commits] master 4234705 2/2: Fits's --write now has title and checksum special keywords

[Mohammad Akhlaghi]

branch: master
commit 42347053aa7bb98b83b27075c2dba2270873175b
Author: Mohammad Akhlaghi <address@hidden>
Commit: Mohammad Akhlaghi <address@hidden>

    Fits's --write now has title and checksum special keywords
    
    Until now, the `--write' option of the FITS program would only write the
    given values to a keyword in the FITS header. But with this commit, we have
    added the infra-structure to include special keywords: for example those
    witout a value (a title), or options where the value must be calculated
    internally (checksum).
    
    This also fixes bug #55333.
---
 NEWS                |  14 ++++-
 bin/fits/args.h     |  13 ++++
 bin/fits/keywords.c | 173 +++++++++++++++++++++++++++++++++++++++++-----------
 bin/fits/main.h     |   1 +
 bin/fits/ui.c       |  91 +++++++++++++++++----------
 bin/fits/ui.h       |   4 +-
 doc/gnuastro.texi   | 108 ++++++++++++++++++++++++++++++--
 7 files changed, 329 insertions(+), 75 deletions(-)

diff --git a/NEWS b/NEWS
index 6e62927..d995c91 100644
--- a/NEWS
+++ b/NEWS
@@ -5,13 +5,25 @@ GNU Astronomy Utilities NEWS                          -*- 
outline -*-
 
 ** New features
 
+  Fits:
+   - Add "title" to group FITS keywords with `--write=/,"title name". This
+     "title" is composed of two keyword records: a blank one, followed by
+     another starting with `/' and ending in any string given to this
+     option. Classifying the keywords into contextually similar groups
+     greatly helps in visual inspection and is ecouraged.
+   - Calculate and write `CHECKSUM' and `DATASUM' integrity keywords into
+     the specified header using `--write=checksum' (for both) or
+     `--write=datasum' (only for `DATASUM').
+   --verify: confirm if the `DATASUM' and `CHECKSUM' keyword values agree
+     with the specified HDU's content and/or data.
+
 ** Removed features
 
 ** Changed features
 
 ** Bugs fixed
   bug #55313: Fits program writing --write values in reverse order
-
+  bug #55333: Fits program crash when --write or --update have no value.
 
 
 
diff --git a/bin/fits/args.h b/bin/fits/args.h
index 372c7c1..b2ecfe0 100644
--- a/bin/fits/args.h
+++ b/bin/fits/args.h
@@ -202,6 +202,19 @@ struct argp_option program_options[] =
       GAL_OPTIONS_NOT_SET
     },
     {
+      "verify",
+      UI_KEY_VERIFY,
+      0,
+      0,
+      "Verify the CHECKSUM and DATASUM keywords.",
+      UI_GROUP_KEYWORD,
+      &p->verify,
+      GAL_OPTIONS_NO_ARG_TYPE,
+      GAL_OPTIONS_RANGE_0_OR_1,
+      GAL_OPTIONS_NOT_MANDATORY,
+      GAL_OPTIONS_NOT_SET
+    },
+    {
       "printallkeys",
       UI_KEY_PRINTALLKEYS,
       0,
diff --git a/bin/fits/keywords.c b/bin/fits/keywords.c
index 366a5fc..27717ad 100644
--- a/bin/fits/keywords.c
+++ b/bin/fits/keywords.c
@@ -123,11 +123,63 @@ keywords_rename_keys(struct fitsparams *p, fitsfile 
**fptr, int *r)
 
 
 
+/* Special write options don't have any value and the value has to be found
+   within the script. */
+static int
+keywords_write_set_value(struct fitsparams *p, fitsfile **fptr,
+                         gal_fits_list_key_t *keyll)
+{
+  int status=0;
+
+  if( !strcasecmp(keyll->keyname,"checksum")
+      || !strcasecmp(keyll->keyname,"datasum") )
+    {
+      /* If a value is given, then just write what the user gave. */
+      if( keyll->value )
+        return 1;
+      else
+        {
+          /* Calculate and write the checksum and datasum. */
+          if( ffpcks(*fptr, &status) )
+            gal_fits_io_error(status, NULL);
+
+          /* If the user just wanted datasum, remove the checksum
+             keyword. */
+          if( !strcasecmp(keyll->keyname,"datasum") )
+            if( fits_delete_key(*fptr, "CHECKSUM", &status) )
+              gal_fits_io_error(status, NULL);
+
+          /* Inform the caller that everything is done. */
+          return 0;
+        }
+    }
+  else if( keyll->keyname[0]=='/' )
+    {
+      gal_fits_key_write_title_in_ptr(keyll->value, *fptr);
+      return 0;
+    }
+  else
+    error(EXIT_FAILURE, 0, "%s: a bug! Please contact us at %s to "
+          "fix the problem. The `keyname' value `%s' is not "
+          "recognized as one with no value", __func__,
+          PACKAGE_BUGREPORT, keyll->keyname);
+
+  /* Function should not reach here. */
+  error(EXIT_FAILURE, 0, "%s: a bug! Please contact us at %s to fix this "
+        "problem. Control should not reach the end of this function",
+        __func__, PACKAGE_BUGREPORT);
+  return 0;
+}
+
+
+
+
+
 static void
 keywords_write_update(struct fitsparams *p, fitsfile **fptr,
                       gal_fits_list_key_t *keyll, int u1w2)
 {
-  int status=0;
+  int status=0, continuewriting=0;
   gal_fits_list_key_t *tmp;
 
   /* Open the FITS file if it hasn't been opened yet. */
@@ -136,54 +188,63 @@ keywords_write_update(struct fitsparams *p, fitsfile 
**fptr,
   /* Go through each key and write it in the FITS file. */
   while(keyll!=NULL)
     {
+      /* Deal with special keywords. */
+      continuewriting=1;
+      if( keyll->value==NULL || keyll->keyname[0]=='/' )
+        continuewriting=keywords_write_set_value(p, fptr, keyll);
+
       /* Write the information: */
-      if(u1w2==1)
+      if(continuewriting)
         {
-          if(keyll->value)
-            {
-              if( fits_update_key(*fptr,
-                                  gal_fits_type_to_datatype(keyll->type),
-                                  keyll->keyname, keyll->value,
-                                  keyll->comment, &status) )
-                gal_fits_io_error(status, NULL);
-            }
-          else
+          if(u1w2==1)
             {
-              if(fits_write_key_null(*fptr, keyll->keyname, keyll->comment,
-                                     &status))
-                gal_fits_io_error(status, NULL);
+              if(keyll->value)
+                {
+                  if( fits_update_key(*fptr,
+                                      gal_fits_type_to_datatype(keyll->type),
+                                      keyll->keyname, keyll->value,
+                                      keyll->comment, &status) )
+                    gal_fits_io_error(status, NULL);
+                }
+              else
+                {
+                  if(fits_write_key_null(*fptr, keyll->keyname,
+                                         keyll->comment, &status))
+                    gal_fits_io_error(status, NULL);
+                }
             }
-        }
-      else if (u1w2==2)
-        {
-          if(keyll->value)
+          else if (u1w2==2)
             {
-              if( fits_write_key(*fptr,
-                                 gal_fits_type_to_datatype(keyll->type),
-                                 keyll->keyname, keyll->value, keyll->comment,
-                                 &status) )
+              if(keyll->value)
+                {
+                  if( fits_write_key(*fptr,
+                                     gal_fits_type_to_datatype(keyll->type),
+                                     keyll->keyname, keyll->value,
+                                     keyll->comment, &status) )
+                    gal_fits_io_error(status, NULL);
+                }
+              else
+                {
+                  if(fits_write_key_null(*fptr, keyll->keyname,
+                                         keyll->comment, &status))
+                    gal_fits_io_error(status, NULL);
+                }
+              if(keyll->unit
+                 && fits_write_key_unit(*fptr, keyll->keyname, keyll->unit,
+                                        &status) )
                 gal_fits_io_error(status, NULL);
             }
           else
-            {
-              if(fits_write_key_null(*fptr, keyll->keyname, keyll->comment,
-                                     &status))
-                gal_fits_io_error(status, NULL);
-            }
+            error(EXIT_FAILURE, 0, "%s: a bug! Please contact us at `%s' so "
+                  "we can fix this problem. The value %d is not valid for "
+                  "`u1w2'", __func__, PACKAGE_BUGREPORT, u1w2);
+
+          /* Add the unit (if one was given). */
           if(keyll->unit
              && fits_write_key_unit(*fptr, keyll->keyname, keyll->unit,
                                     &status) )
             gal_fits_io_error(status, NULL);
         }
-      else
-        error(EXIT_FAILURE, 0, "%s: a bug! Please contact us at `%s' so we "
-              "can fix this problem. The value %d is not valid for `u1w2'",
-              __func__, PACKAGE_BUGREPORT, u1w2);
-
-      /* Add the unit (if one was given). */
-      if(keyll->unit
-         && fits_write_key_unit(*fptr, keyll->keyname, keyll->unit, &status) )
-        gal_fits_io_error(status, NULL);
 
       /* Free the allocated spaces if necessary: */
       if(keyll->vfree) free(keyll->value);
@@ -237,6 +298,38 @@ keywords_print_all_keys(struct fitsparams *p, fitsfile 
**fptr)
 
 
 
+static int
+keywords_verify(struct fitsparams *p, fitsfile **fptr)
+{
+  int dataok, hduok, status=0;
+
+  /* Ask CFITSIO to verify the two keywords. */
+  if( fits_verify_chksum(*fptr, &dataok, &hduok, &status) )
+    gal_fits_io_error(status, NULL);
+
+  /* Print the verification result. */
+  printf("DATASUM:  %s\n", ( dataok==1
+                             ? "Verified"
+                             : ( dataok==0 ? "NOT-PRESENT" : "INCORRECT" )));
+  printf("CHECKSUM: %s\n", ( hduok==1
+                             ? "Verified"
+                             : ( hduok==0  ? "NOT-PRESENT" : "INCORRECT" )));
+
+  /* Some further information. */
+  if(!p->cp.quiet)
+    printf("\n--------\n"
+           " - DATASUM:  calculated only from the HDU/extension's data (not "
+           "keywords).\n"
+           " - CHECKSUM: calculated from the full header (data and "
+           "keywords).\n\n");
+
+  /* Return failure if any of the keywords are not verified. */
+  return (dataok==-1 || hduok==-1) ? EXIT_FAILURE : EXIT_SUCCESS;
+}
+
+
+
+
 
 
 
@@ -373,6 +466,14 @@ keywords(struct fitsparams *p)
     }
 
 
+  /* Verify the CHECKSUM and DATASUM keys. */
+  if(p->verify)
+    {
+      keywords_open(p, &fptr, READONLY);
+      r=keywords_verify(p, &fptr);
+    }
+
+
   /* Close the FITS file */
   if(fits_close_file(fptr, &status))
     gal_fits_io_error(status, NULL);
diff --git a/bin/fits/main.h b/bin/fits/main.h
index 43edc06..78f0054 100644
--- a/bin/fits/main.h
+++ b/bin/fits/main.h
@@ -69,6 +69,7 @@ struct fitsparams
   gal_list_str_t   *write;     /* Full arg. for keywords to add.        */
   gal_list_str_t *history;     /* HISTORY value.                        */
   gal_list_str_t *comment;     /* COMMENT value.                        */
+  uint8_t         *verify;     /* Verify the CHECKSUM and DATASUM keys. */
   uint8_t     quitonerror;     /* Quit if an error occurs.              */
 
   /* Internal: */
diff --git a/bin/fits/ui.c b/bin/fits/ui.c
index 8e6dce7..1674865 100644
--- a/bin/fits/ui.c
+++ b/bin/fits/ui.c
@@ -225,7 +225,7 @@ ui_read_check_only_options(struct fitsparams *p)
   /* If any of the keyword manipulation options are requested, then set the
      mode flag to keyword-mode. */
   if( p->date || p->comment || p->history || p->asis || p->delete
-      || p->rename || p->update || p->write || p->printallkeys )
+      || p->rename || p->update || p->write || p->verify || p->printallkeys )
     {
       /* Set the mode. */
       p->mode=FITS_MODE_KEY;
@@ -315,14 +315,15 @@ ui_check_options_and_arguments(struct fitsparams *p)
    keyword, so here, we tokenize them and put them into a
    `gal_fits_list_key_t' list. */
 static void
-ui_fill_fits_headerll(gal_list_str_t *input, gal_fits_list_key_t **output)
+ui_fill_fits_headerll(gal_list_str_t *input, gal_fits_list_key_t **output,
+                      char *option_name)
 {
   long l, *lp;
   void *fvalue;
   double d, *dp;
   gal_list_str_t *tmp;
-  int i=0, type, vfree;
   char *c, *cf, *start, *tailptr;
+  int i=0, type, vfree, needsvalue;
   char *original, *keyname, *value, *comment, *unit;
 
   for(tmp=input; tmp!=NULL; tmp=tmp->next)
@@ -380,50 +381,74 @@ ui_fill_fits_headerll(gal_list_str_t *input, 
gal_fits_list_key_t **output)
             }
         }
       while(++c<cf);
-      if(keyname==NULL)
-        error(EXIT_FAILURE, 0, "the keyword in %s was not readable. "
-              "The general expected format is:\n"
-              "    KEYWORD,value,\"a comment string\",unit\n"
+
+      /* See if this is an option that needs a value or not.*/
+      needsvalue=1;
+      if(keyname)
+        {
+          if( !strcasecmp(keyname,"checksum")
+              || !strcasecmp(keyname,"datasum") )
+            needsvalue=0;
+        }
+
+      /* Make sure the keyname and value (if necessary) is given. */
+      if( keyname==NULL || (needsvalue && value==NULL) )
+        error(EXIT_FAILURE, 0, "`--%s' option string (%s) can't be parsed. "
+              "The general expected format is (a comment string and unit "
+              "are optional):\n\n"
+              "    --%s=KEYWORD,value,\"a comment string\",unit\n\n"
               "Any space characters around the the comma (,) characters "
-              "will be seen as part of the respective token", original);
+              "will be seen as part of the respective token.\n\n"
+              "Note that there are some exceptions (where no value is need)"
+              "please see the manual for more (`$ info astfits')",
+              option_name, original, option_name);
       /*
       printf("\n\n-%s-\n-%s-\n-%s-\n-%s-\n", keyname, value, comment, unit);
       */
 
-      /* Find the of the value: */
-      errno=0;
-      l=strtol(value, &tailptr, 10);
-      if(*tailptr=='\0' && errno==0)
-        {
-          vfree=1;
-          type=GAL_TYPE_INT64;
-          errno=0;
-          fvalue=lp=malloc(sizeof *lp);
-          if(lp==NULL)
-            error(EXIT_FAILURE, errno, "%s: %zu bytes for `lp'",
-                  __func__, sizeof *lp);
-          *lp=l;
-        }
-      else
+
+      /* Find the type of the value: */
+      if(value)
         {
           errno=0;
-          d=strtod(value, &tailptr);
+          l=strtol(value, &tailptr, 10);
           if(*tailptr=='\0' && errno==0)
             {
               vfree=1;
-              type=GAL_TYPE_FLOAT64;
+              type=GAL_TYPE_INT64;
               errno=0;
-              fvalue=dp=malloc(sizeof *dp);
-              if(dp==NULL)
-                error(EXIT_FAILURE, errno, "%s: allocating %zu bytes for "
-                      "`dp'", __func__, sizeof *dp);
-              *dp=d;
+              fvalue=lp=malloc(sizeof *lp);
+              if(lp==NULL)
+                error(EXIT_FAILURE, errno, "%s: %zu bytes for `lp'",
+                      __func__, sizeof *lp);
+              *lp=l;
             }
           else
-            { fvalue=value; type=GAL_TYPE_STRING; vfree=0; }
+            {
+              errno=0;
+              d=strtod(value, &tailptr);
+              if(*tailptr=='\0' && errno==0)
+                {
+                  vfree=1;
+                  type=GAL_TYPE_FLOAT64;
+                  errno=0;
+                  fvalue=dp=malloc(sizeof *dp);
+                  if(dp==NULL)
+                    error(EXIT_FAILURE, errno, "%s: allocating %zu bytes "
+                          "for `dp'", __func__, sizeof *dp);
+                  *dp=d;
+                }
+              else
+                { fvalue=value; type=GAL_TYPE_STRING; vfree=0; }
+            }
+        }
+      else
+        {
+          fvalue=NULL; type=GAL_TYPE_UINT8; vfree=0;
         }
 
 
+      /* Add it to the list of keywords. */
       gal_fits_key_list_add(output, type, keyname, 0, fvalue, vfree,
                             comment, 0, unit);
       free(original);
@@ -443,8 +468,8 @@ ui_preparations(struct fitsparams *p)
   /* Fill in the key linked lists. We want to do this here so if there is
      any error in parsing the user's input, the error is reported before
      any change is made in the input file. */
-  if(p->write)  ui_fill_fits_headerll(p->write, &p->write_keys);
-  if(p->update) ui_fill_fits_headerll(p->update, &p->update_keys);
+  if(p->write)  ui_fill_fits_headerll(p->write, &p->write_keys, "write");
+  if(p->update) ui_fill_fits_headerll(p->update, &p->update_keys, "update");
 }
 
 
diff --git a/bin/fits/ui.h b/bin/fits/ui.h
index be7fccf..3df455a 100644
--- a/bin/fits/ui.h
+++ b/bin/fits/ui.h
@@ -43,7 +43,7 @@ enum program_args_groups
 
 /* Available letters for short options:
 
-   b e f g i j l m s v x y z
+   b e f g i j l m s x y z
    A B E G J L O W X Y
  */
 enum option_keys_enum
@@ -62,11 +62,13 @@ enum option_keys_enum
   UI_KEY_COMMENT      = 'c',
   UI_KEY_HISTORY      = 'H',
   UI_KEY_DATE         = 't',
+  UI_KEY_VERIFY       = 'v',
   UI_KEY_QUITONERROR  = 'Q',
 
 
   /* Only with long version (start with a value 1000, the rest will be set
      automatically). */
+  UI_KEY_TITLE        = 1000,
 };
 
 
diff --git a/doc/gnuastro.texi b/doc/gnuastro.texi
index 7cb2e17..3638245 100644
--- a/doc/gnuastro.texi
+++ b/doc/gnuastro.texi
@@ -6150,9 +6150,13 @@ segmentation fault is found during @command{make check}.
 Only build and install @file{progname} along with any other program that is
 enabled in this fashion. @file{progname} is the name of the executable
 without the @file{ast}, for example @file{crop} for Crop (with the
-executable name of @file{astcrop}). If this option is called for any of the
-programs in Gnuastro, any program which is not explicitly enabled will not
-be built or installed.
+executable name of @file{astcrop}).
+
+Note that by default all the programs will be installed. This option (and
+the @option{--disable-progname} options) are only relevant when you don't
+want to install all the programs. Therefore, if this option is called for
+any of the programs in Gnuastro, any program which is not explicitly
+enabled will not be built or installed.
 
 @item --disable-progname
 @itemx --enable-progname=no
@@ -10140,6 +10144,8 @@ effect then the update operations.
 @option{--date}
 @item
 @option{--printallkeys}
address@hidden
address@hidden
 @end enumerate
 @noindent
 All possible syntax errors will be reported before the keywords are
@@ -10269,7 +10275,89 @@ lead to undefined behavior.
 @item -w STR
 @itemx --write=STR
 Write a keyword to the header. For the possible value input formats,
-comments and units for the keyword, see the @option{--update} option above.
+comments and units for the keyword, see the @option{--update} option
+above. The special names (first string) below will cause a special
+behavior:
+
address@hidden @option
+
address@hidden /
+Write a ``title'' to the list of keywords. A title consists of one blank
+line and another which is blank for several spaces and starts with a slash
+(@key{/}). The second string given to this option is the ``title'' or
+string printed after the slash. For example with the command below you can
+add a ``title'' of `My keywords' after the existing keywords and add the
+subsequent @code{K1} and @code{K2} keywords under it (note that keyword
+names are not case sensitive).
+
address@hidden
+$ astfits test.fits -h1 --write=/,"My keywords" \
+          --write=k1,1.23,"My first keyword"    \
+          --write=k2,4.56,"My second keyword"
+$ astfits test.fits -h1
+[[[ ... truncated ... ]]]
+
+                      / My keywords
+K1      =                 1.23 / My first keyword
+K2      =                 4.56 / My second keyword
+END
address@hidden example
+
+Adding a ``title'' before each contextually separate group of header
+keywords greatly helps in readability and visual inspection of the
+keywords. So generally, when you want to add new FITS keywords, its good
+practice to also add a title before them.
+
+The reason you need to use @key{/} as the keyword name for setting a title
+is that @key{/} is the first non-white character.
+
+The title(s) is(are) written into the FITS with the same order that
address@hidden is called. Therefore in one run of the Fits progarm, you
+can specify many different titles (with their own keywords under them). For
+example the command below that builds on the previous example and adds
+another group of keywords named @code{A1} and @code{A2}.
+
address@hidden
+$ astfits test.fits -h1 --write=/,"My keywords"   \
+          --write=k1,1.23,"My first keyword"      \
+          --write=k2,4.56,"My second keyword"     \
+          --write=/,"My second group of keywords" \
+          --write=a1,7.89,"First keyword"         \
+          --write=a2,0.12,"Second keyword"
address@hidden example
+
address@hidden checksum
address@hidden CFITSIO
address@hidden @code{DATASUM}: FITS keyword
address@hidden @code{CHECKSUM}: FITS keyword
+When nothing is given afterwards, the header integrity
address@hidden 4.4.2.7 (page 15) of
address@hidden://fits.gsfc.nasa.gov/standard40/fits_standard40aa-le.pdf}}
address@hidden and @code{CHECKSUM} will be written/updated. They are
+calculated and written by CFITSIO. They thus comply with the FITS standard
+4.0 that defines these keywords.
+
+If a value is given (for example
address@hidden,my-own-checksum,"my checksum"}), then CFITSIO
+won't be called to calculate these two keywords and the value (as well as
+possible comment and unit) will be written just like any other
+keyword. This is generally not recommended, but necessary in special
+circumstances (where the checksum needs to be manually updated for
+example).
+
address@hidden only depends on the data section of the HDU/extension, so it
+is not changed when you update the keywords. But @code{CHECKSUM} also
+depends on the header and will not be valid if you make any further changes
+to the header. This includes any further keyword modification options in
+the same call to the Fits program. Therefore it is recommended to write
+these keywords as the last keywords that are written/modified in the
+extension. You can use the @option{--verify} option (described below) to
+verify the values of these two keywords.
+
address@hidden datasum
+Similar to @option{checksum}, but only write the @code{DATASUM} keyword
+(that doesn't depend on the header keywords, only the data).
address@hidden table
 
 @item -H STR
 @itemx --history STR
@@ -10301,6 +10389,18 @@ editing commands, as described above, all other 
editing commands take
 precedence to this. Therefore, it will print the final keywords after all
 the editing has been done.
 
address@hidden -v
address@hidden --verify
+Verify the @code{DATASUM} and @code{CHECKSUM} data integrity keywords of
+the FITS standard. See the description under the @code{checksum} (under
address@hidden, above) for more on these keywords.
+
+This option will print @code{Verified} for both keywords if they can be
+verified. Otherwise, if they don't exist in the given HDU/extension, it
+will print @code{NOT-PRESENT}, and if they cannot be verified it will print
address@hidden In the latter case (when the keyword values exist but
+can't be verified), the Fits program will also return with a failure.
+
 @item -Q
 @itemx --quitonerror
 Quit if any of the operations above are not successful. By default if