[svn-r18796] Enhancement 1853:

Install packed-bits feature for h5dump.

tools/h5dump/h5dump.c:
    Added lots of error checking into parse_mask_list() to catch and
    flagged packed-bits argument errors. For now, the packed bits
    are limited to 1 byte (8 bits) range.

tools/h5dump/testh5dump.sh.in
    Added 10 new packed-bits test to verify normal operation and argument
    error detection.

tools/lib/h5tools.h:
tools/lib/h5tools_str.c:
tools/lib/h5tools.c:
    Only name changes of the packed-bit global variables to reflect
    the nature of the variable better, IMO.

Tested: albertPax (linux)
------------------------------------------------------------------------

5 files changed, 155 insertions, 92 deletions

diff --git a/tools/h5dump/h5dump.c b/tools/h5dump/h5dump.c
index 49bcb8f..a9adeb5 100644
--- a/tools/h5dump/h5dump.c
+++ b/tools/h5dump/h5dump.c
@@ -101,13 +101,14 @@ static H5_index_t   sort_by           = H5_INDEX_NAME; /*sort_by [creation_order
 static H5_iter_order_t sort_order     = H5_ITER_INC; /*sort_order [ascending | descending]   */
 
 #ifdef H5_HAVE_H5DUMP_PACKED_BITS
-#define PACKED_BITS_LOOP_MAX         8
+#define PACKED_BITS_MAX         8	/* Maximum number of packed-bits to display */
+#define PACKED_BITS_SIZE_MAX    8	/* Maximum bits size of integer types of packed-bits */
 /* mask list for packed bits */
-static unsigned int packed_mask[PACKED_BITS_LOOP_MAX];  /* packed bits are restricted to 1 byte */
+static unsigned int packed_mask[PACKED_BITS_MAX];  /* packed bits are restricted to 1 byte */
 
 /* packed bits display parameters */
-static int packed_offset[PACKED_BITS_LOOP_MAX];
-static int packed_length[PACKED_BITS_LOOP_MAX];
+static int packed_offset[PACKED_BITS_MAX];
+static int packed_length[PACKED_BITS_MAX];
 #endif
 
 /**
@@ -402,11 +403,14 @@ struct handler_t {
  * parameters. The long-named ones can be partially spelled. When
  * adding more, make sure that they don't clash with each other.
  */
+/* The following initialization makes use of C language cancatenating */
+/* "xxx" "yyy" into "xxxyyy". */
+static const char *s_opts = "hnpeyBHirVa:c:d:f:g:k:l:t:w:xD:uX:o:b*F:s:S:Aq:z:m:R"
 #ifdef H5_HAVE_H5DUMP_PACKED_BITS
-static const char *s_opts = "hnpeyBHirVa:c:d:f:g:k:l:t:w:xD:uX:o:b*F:s:S:Aq:z:m:RM:";
-#else
-static const char *s_opts = "hnpeyBHirVa:c:d:f:g:k:l:t:w:xD:uX:o:b*F:s:S:Aq:z:m:R";
+"M:"
 #endif
+;	/* end of *s_opt initialization */
+
 static struct long_options l_opts[] = {
     { "help", no_arg, 'h' },
     { "hel", no_arg, 'h' },
@@ -2243,11 +2247,6 @@ dump_dataset(hid_t did, const char *name, struct subset_t *sset)
     hid_t       type, space;
     unsigned    attr_crt_order_flags;
     hid_t       dcpl_id;  /* dataset creation property list ID */
-#ifdef H5_HAVE_H5DUMP_PACKED_BITS
-    int         data_loop = 1;
-    int         i;
-#endif
-
 
     if ((dcpl_id = H5Dget_create_plist(did)) < 0)
     {
@@ -2281,8 +2280,10 @@ dump_dataset(hid_t did, const char *name, struct subset_t *sset)
 
     if(display_data) {
 #ifdef H5_HAVE_H5DUMP_PACKED_BITS
+	int	data_loop = 1;
+	int	i;
         if(display_packed_bits)
-            data_loop = packed_output;
+            data_loop = packed_bits_num;
         for(i=0;i<data_loop;i++) {
         if(display_packed_bits) {
             dump_packed_bits(i);
@@ -3592,78 +3593,114 @@ parse_subset_params(char *dset)
  *              the packed_bits list and counter. The string being passed into this function
  *              should be at the start of the list you want to parse. 
  *
- * Return:      None
+ * Return:      Success:        SUCCEED
+ *
+ *              Failure:        FAIL
+ *
  *
  *-------------------------------------------------------------------------
  */
-static void
+static int
 parse_mask_list(const char *h_list)
 {
     const char     *ptr;
-    unsigned int    size_count = 0, i = 0, last_digit = 0;
     int             offset_value = 0, length_value = 0;
 
+    /* sanity check */
+    HDassert(h_list);
+
     packed_counter = 0;
-    memset(packed_mask,0,8);
-    memset(packed_offset,0,8);
-    memset(packed_length,0,8);
+    HDmemset(packed_mask,0,sizeof(packed_mask));
+    HDmemset(packed_offset,0,sizeof(packed_offset));
+    HDmemset(packed_length,0,sizeof(packed_length));
     
-    if (!h_list || !*h_list || *h_list == ';')
-        return;
-
-    /* count how many integers do we have */
-    for (ptr = h_list; ptr && *ptr && *ptr != ';' && *ptr != ']'; ptr++) {
-        if (isdigit(*ptr)) {
-            if (!last_digit)
-                /* the last read character wasn't a digit */
-                size_count++;
-
-            last_digit = 1;
-        } 
-        else {
-            last_digit = 0;
-        }
-    }
-
-    if (size_count == 0) 
-        /* there aren't any integers to read */
-       return;
-
     offset_value = -1;
     length_value = -1;
-    packed_output = 0;
-    for (ptr = h_list; i < size_count && ptr && *ptr && *ptr != ';' && *ptr != ']'; ptr++) {
-        if(isdigit(*ptr)) {
-            i++;
-            /* we should have an integer now */
-            if(offset_value==-1)
-                offset_value = atoi(ptr);
-            else
-                length_value = atoi(ptr);
-
-            while (isdigit(*ptr))
-                /* scroll to end of integer */
-                ptr++;
-        }
-        if(length_value>=0) {
-            packed_offset[packed_output] = offset_value;
-            packed_length[packed_output] = length_value;
-            
-            packed_mask[packed_output] = 1 << offset_value;
-            while(length_value>1) {
-                packed_mask[packed_output] = packed_mask[packed_output] << 1;
-                packed_mask[packed_output] |= 1 << offset_value;
-                length_value--;
-            }
-            packed_output++;
-            offset_value = -1;
-            length_value = -1;
-        }
+    packed_bits_num = 0;
+    /* scan in pair of offset,length separated by commas. */
+    ptr = h_list;
+    while (*ptr) {
+	/* scan for an offset which is an unsigned int */
+	if (!HDisdigit(*ptr)){
+	    error_msg(h5tools_getprogname(), "Bad mask list(%s)\n", h_list);
+	    return FAIL;
+	}
+	offset_value = HDatoi(ptr);
+	if (offset_value < 0 || offset_value >= PACKED_BITS_SIZE_MAX){
+	    error_msg(h5tools_getprogname(), "Packed Bit offset value(%d) must be between 0 and %d\n",
+		offset_value, PACKED_BITS_SIZE_MAX - 1);
+	    return FAIL;
+	}
+
+	/* skip to end of integer */
+	while (HDisdigit(*++ptr))
+	    ;
+	/* Look for the common separator */
+	if (*ptr++ != ',') {
+	    error_msg(h5tools_getprogname(), "Bad mask list(%s), missing expected comma separator.\n", h_list);
+	    return FAIL;
+	}
+
+	/* scan for a length which is a positive int */
+	if (!HDisdigit(*ptr)){
+	    error_msg(h5tools_getprogname(), "Bad mask list(%s)\n", h_list);
+	    return FAIL;
+	}
+	length_value = HDatoi(ptr);
+	if (length_value <= 0){
+	    error_msg(h5tools_getprogname(), "Packed Bit length value(%d) must be positive.\n",
+		length_value);
+	    return FAIL;
+	};
+	if ((offset_value + length_value) > PACKED_BITS_SIZE_MAX){
+	    error_msg(h5tools_getprogname(), "Packed Bit offset+length value(%d) too large. Max is %d\n",
+		offset_value+length_value, PACKED_BITS_SIZE_MAX);
+	    return FAIL;
+	};
+
+	/* skip to end of int */
+	while (HDisdigit(*++ptr))
+	    ;
+
+	/* store the offset,length pair */
+	if (packed_bits_num >= PACKED_BITS_MAX){
+	    /* too many requests */
+	    error_msg(h5tools_getprogname(), "Too many masks requested (max. %d). Mask list(%s)\n",
+		PACKED_BITS_MAX, h_list);
+	    return FAIL;
+	}
+	packed_offset[packed_bits_num] = offset_value;
+	packed_length[packed_bits_num] = length_value;
+	
+	packed_mask[packed_bits_num] = 1 << offset_value;
+	while(length_value>1) {
+	    packed_mask[packed_bits_num] = packed_mask[packed_bits_num] << 1;
+	    packed_mask[packed_bits_num] |= 1 << offset_value;
+	    length_value--;
+	}
+	packed_bits_num++;
+	offset_value = -1;
+	length_value = -1;
+
+	/* skip a possible comma separator */
+	if (*ptr == ','){
+	    if (!(*++ptr)){
+		/* unexpected end of string */
+		error_msg(h5tools_getprogname(), "Bad mask list(%s), unexpected end of string.\n", h_list);
+		return FAIL;
+	    }
+	}
+    }
+    HDassert(packed_bits_num <= PACKED_BITS_MAX);
+    if (packed_bits_num == 0){
+	/* got no masks! */
+        error_msg(h5tools_getprogname(), "Bad mask list(%s)\n", h_list);
+        return FAIL;
     }
-    if(packed_output > PACKED_BITS_LOOP_MAX)
-        packed_output = PACKED_BITS_LOOP_MAX;
     packed_counter = packed_mask[0];
+    return SUCCEED;
 }
+
 #endif
 
 /*-------------------------------------------------------------------------
@@ -4122,7 +4159,7 @@ parse_start:
             leave(EXIT_SUCCESS);
             break;
         case 'w':
-            nCols = atoi(opt_arg);
+            nCols = HDatoi(opt_arg);
             last_was_dset = FALSE;
             break;
         case 'a':
@@ -4254,7 +4291,10 @@ parse_start:
                           opt);
                 leave(EXIT_FAILURE);
             }
-            parse_mask_list(opt_arg);
+            if (parse_mask_list(opt_arg) != SUCCEED){
+		usage(h5tools_getprogname());
+                leave(EXIT_FAILURE);
+            }
             display_packed_bits = TRUE;
             break;
 #endif
diff --git a/tools/h5dump/testh5dump.sh.in b/tools/h5dump/testh5dump.sh.in
index 7776b62..e8145ce 100644
--- a/tools/h5dump/testh5dump.sh.in
+++ b/tools/h5dump/testh5dump.sh.in
@@ -488,8 +488,6 @@ if test -z "$HDF5_NOCLEANUP"; then
  rm -f $TESTDIR/out3.h5
 fi
 
-
-
 # test for dataset region references 
 TOOLTEST tdatareg.ddl tdatareg.h5
 TOOLTEST tdataregR.ddl -R tdatareg.h5
@@ -510,7 +508,6 @@ TOOLTEST torderattr2.ddl -H --sort_by=name --sort_order=descending torderattr.h5
 TOOLTEST torderattr3.ddl -H --sort_by=creation_order --sort_order=ascending torderattr.h5
 TOOLTEST torderattr4.ddl -H --sort_by=creation_order --sort_order=descending torderattr.h5
 
-
 # tests for floating point user defined printf format
 TOOLTEST tfpformat.ddl -m %.7f tfpformat.h5
 
@@ -518,17 +515,43 @@ TOOLTEST tfpformat.ddl -m %.7f tfpformat.h5
 TOOLTEST textlinksrc.ddl textlinksrc.h5
 TOOLTEST textlinkfar.ddl textlinkfar.h5
 
-
 # test for dataset packed bits 
+# Set up xCMD to test or skip.
 if test "$Have_Packed_Bits" = "yes"; then
-    TOOLTEST tpackedbits.ddl -d /dset1 -M 0,2 tdset.h5
-    TOOLTEST tpackedbits2.ddl -d /dset1 -M 0,2,2,1 tdset.h5
+    xCMD=TOOLTEST
 else
-    SKIP tpackedbits.ddl -d /dset1 -M 0,2 tdset.h5
-    SKIP tpackedbits2.ddl -d /dset1 -M 0,2,2,1 tdset.h5
+    xCMD=SKIP
 fi
-
-
+# Limits:
+# Maximum number of packed bits is 8 (for now).
+# Maximum integer size is 8 (for now).
+# Maximun Offset is 7 (Maximum size - 1).
+# Maximum Offset+Length is 8 (Maximum size).
+# Tests:
+# Normal operation on both signed and unsigned int datasets.
+# Their rawdata output should be the same.
+$xCMD tpbitsSigned.ddl -d /DS08BITS -M 0,2,2,6 packedbits.h5
+$xCMD tpbitsUnsigned.ddl -d /DU08BITS -M 0,2,2,6 packedbits.h5
+# Overlapped packed bits.
+$xCMD tpbitsOverlapped.ddl -d /DS08BITS -M 0,1,1,1,2,1,0,3 packedbits.h5
+# Maximum number of packed bits.
+$xCMD tpbitsMax.ddl -d /DS08BITS -M 0,1,1,1,2,1,3,1,4,1,5,1,6,1,7,1 packedbits.h5
+# Test Error handling.
+# Too many packed bits requested. Max is 8 for now.
+$xCMD tpbitsMaxExceeded.ddl -d /DS08BITS -M 0,1,0,1,1,1,2,1,3,1,4,1,5,1,6,1,7,1 packedbits.h5
+# Offset too large. Max is 7 (8-1) for now.
+$xCMD tpbitsOffsetExceeded.ddl -d /DS08BITS -M 8,1 packedbits.h5
+# Bad offset, must not be negative.
+$xCMD tpbitsOffsetNegative.ddl -d /DS08BITS -M -1,1 packedbits.h5
+# Bad length, must not be positive.
+$xCMD tpbitsLengthPositive.ddl -d /DS08BITS -M 4,0 packedbits.h5
+# Offset+Length is too large. Max is 8 for now.
+$xCMD tpbitsLengthExceeded.ddl -d /DS08BITS -M 2,7 packedbits.h5
+# Incomplete pair of packed bits request.
+$xCMD tpbitsIncomplete.ddl -d /DS08BITS -M 0,2,2,1,0,2,2, packedbits.h5
+
+
+# Report test results and exit
 if test $nerrors -eq 0 ; then
     echo "All $TESTNAME tests passed."
     exit $EXIT_SUCCESS
diff --git a/tools/lib/h5tools.c b/tools/lib/h5tools.c
index ddfccf8..ff0437c 100644
--- a/tools/lib/h5tools.c
+++ b/tools/lib/h5tools.c
@@ -45,7 +45,7 @@ int         region_output; /* region output */
 static int  h5tools_d_status = 0;
 static const char  *h5tools_progname = "h5tools";
 #ifdef H5_HAVE_H5DUMP_PACKED_BITS
-int         packed_output; /* number of packed bits to display */
+int         packed_bits_num; /* number of packed bits to display */
 int         packed_normalize; /* number of bits to shift right to display normalized */
 unsigned int packed_counter;  /* counter for which packed bits to display */
 #endif
diff --git a/tools/lib/h5tools.h b/tools/lib/h5tools.h
index 16dabd0..9d2d9e6 100644
--- a/tools/lib/h5tools.h
+++ b/tools/lib/h5tools.h
@@ -526,7 +526,7 @@ extern "C" {
 #endif
     
 #ifdef H5_HAVE_H5DUMP_PACKED_BITS
-H5TOOLS_DLLVAR int     packed_output;       /* packed bits output count */
+H5TOOLS_DLLVAR int     packed_bits_num;       /* number of packed bits to display */
 H5TOOLS_DLLVAR int     packed_normalize; /* number of bits to shift right to display normalized */
 H5TOOLS_DLLVAR unsigned int  packed_counter;      /* counter for which packed bits to display */
 #endif
diff --git a/tools/lib/h5tools_str.c b/tools/lib/h5tools_str.c
index c7fb8a9..b1c017a 100644
--- a/tools/lib/h5tools_str.c
+++ b/tools/lib/h5tools_str.c
@@ -784,7 +784,7 @@ h5tools_str_sprint(h5tools_str_t *str, const h5tool_format_t *info, hid_t contai
     else if (H5Tequal(type, H5T_NATIVE_INT)) {
         HDmemcpy(&tempint, vp, sizeof(int));
 #ifdef H5_HAVE_H5DUMP_PACKED_BITS
-        if(packed_output)
+        if(packed_bits_num)
             tempint = (tempint & packed_counter)>>packed_normalize;
 #endif
         h5tools_str_append(str, OPT(info->fmt_int, "%d"), tempint);
@@ -792,7 +792,7 @@ h5tools_str_sprint(h5tools_str_t *str, const h5tool_format_t *info, hid_t contai
     else if (H5Tequal(type, H5T_NATIVE_UINT)) {
         HDmemcpy(&tempuint, vp, sizeof(unsigned int));
 #ifdef H5_HAVE_H5DUMP_PACKED_BITS
-        if(packed_output)
+        if(packed_bits_num)
             tempuint = (tempuint & packed_counter)>>packed_normalize;
 #endif
         h5tools_str_append(str, OPT(info->fmt_uint, "%u"), tempuint);
@@ -801,7 +801,7 @@ h5tools_str_sprint(h5tools_str_t *str, const h5tool_format_t *info, hid_t contai
         char               tempchar;
         HDmemcpy(&tempchar, cp_vp, sizeof(char));
 #ifdef H5_HAVE_H5DUMP_PACKED_BITS
-        if(packed_output)
+        if(packed_bits_num)
             tempchar = (tempchar & packed_counter)>>packed_normalize;
 #endif
         h5tools_str_append(str, OPT(info->fmt_schar, "%d"), tempchar);
@@ -810,7 +810,7 @@ h5tools_str_sprint(h5tools_str_t *str, const h5tool_format_t *info, hid_t contai
         unsigned char      tempuchar;
         HDmemcpy(&tempuchar, ucp_vp, sizeof(unsigned char));
 #ifdef H5_HAVE_H5DUMP_PACKED_BITS
-        if(packed_output)
+        if(packed_bits_num)
             tempuchar = (tempuchar & packed_counter)>>packed_normalize;
 #endif
         h5tools_str_append(str, OPT(info->fmt_uchar, "%u"), tempuchar);
@@ -820,7 +820,7 @@ h5tools_str_sprint(h5tools_str_t *str, const h5tool_format_t *info, hid_t contai
 
         HDmemcpy(&tempshort, vp, sizeof(short));
 #ifdef H5_HAVE_H5DUMP_PACKED_BITS
-        if(packed_output)
+        if(packed_bits_num)
             tempshort = (tempshort & packed_counter)>>packed_normalize;
 #endif
         h5tools_str_append(str, OPT(info->fmt_short, "%d"), tempshort);
@@ -830,7 +830,7 @@ h5tools_str_sprint(h5tools_str_t *str, const h5tool_format_t *info, hid_t contai
 
         HDmemcpy(&tempushort, vp, sizeof(unsigned short));
 #ifdef H5_HAVE_H5DUMP_PACKED_BITS
-        if(packed_output)
+        if(packed_bits_num)
             tempushort = (tempushort & packed_counter)>>packed_normalize;
 #endif
         h5tools_str_append(str, OPT(info->fmt_ushort, "%u"), tempushort);
@@ -838,7 +838,7 @@ h5tools_str_sprint(h5tools_str_t *str, const h5tool_format_t *info, hid_t contai
     else if (H5Tequal(type, H5T_NATIVE_LONG)) {
         HDmemcpy(&templong, vp, sizeof(long));
 #ifdef H5_HAVE_H5DUMP_PACKED_BITS
-        if(packed_output)
+        if(packed_bits_num)
             templong = (templong & packed_counter)>>packed_normalize;
 #endif
         h5tools_str_append(str, OPT(info->fmt_long, "%ld"), templong);
@@ -846,7 +846,7 @@ h5tools_str_sprint(h5tools_str_t *str, const h5tool_format_t *info, hid_t contai
     else if (H5Tequal(type, H5T_NATIVE_ULONG)) {
         HDmemcpy(&tempulong, vp, sizeof(unsigned long));
 #ifdef H5_HAVE_H5DUMP_PACKED_BITS
-        if(packed_output)
+        if(packed_bits_num)
             tempulong = (tempulong & packed_counter)>>packed_normalize;
 #endif
         h5tools_str_append(str, OPT(info->fmt_ulong, "%lu"), tempulong);