Refactor error handling code to eliminate internal ID calls (HDFGroup…

…#4453) All calls to the H5I routines are now made in API routines (sometimes in FUNC_ENTER/LEAVE_* macros), except for some calls to H5E_clear_stack() within the library, but I'm planning to remove those over time. Also, made all the library internal error messages into static const variables, instead of malloc'ing them, which means that they can just be referenced and not copied. Several new and updated auto-generated header files were necessary to enable this.
byrnHDF · May 14, 2024 · 4fdf3b6 · 4fdf3b6
1 parent 040f910
commit 4fdf3b6
Show file tree

Hide file tree

Showing 32 changed files with 1,265 additions and 1,108 deletions.
diff --git a/.gitignore b/.gitignore
@@ -34,6 +34,8 @@ m4/ltversion.m4
 m4/lt~obsolete.m4
 src/H5Edefin.h
 src/H5Einit.h
+src/H5Emajdef.h
+src/H5Emindef.h
 src/H5Epubgen.h
 src/H5Eterm.h
 src/H5config.h.in

diff --git a/bin/h5vers b/bin/h5vers
@@ -95,6 +95,8 @@ sub setvers {
   $_[0] =~ s/^(\#\s*define\s+H5_VERS_MINOR\s+)\d+/$1$vers[1]/m;
   $_[0] =~ s/^(\#\s*define\s+H5_VERS_RELEASE\s+)\d+/$1$vers[2]/m;
   $_[0] =~ s/^(\#\s*define\s+H5_VERS_SUBRELEASE\s+\")[^\"]*/$1$vers[3]/m;
+  $_[0] =~ s/^(\#\s*define\s+H5_VERS_STR\s+\")[^\"]*/
+    sprintf("%s%d.%d.%d%s%s", $1, @vers[0,1,2], $vers[3]?"-":"", $vers[3])/me;
   $_[0] =~ s/^(\#\s*define\s+H5_VERS_INFO\s+\")[^\"]*/
     sprintf("%sHDF5 library version: %d.%d.%d%s%s", $1, @vers[0,1,2],
 	    $vers[3]?"-":"", $vers[3])/me;

diff --git a/bin/make_err b/bin/make_err
@@ -47,7 +47,7 @@ sub print_warning ($) {
     my $fh = shift;
 
     print $fh "\n/* Generated automatically by bin/make_err -- do not edit */\n";
-    print $fh "/* Add new errors to H5err.txt file */\n\n";
+    print $fh "/* Add new errors to H5err.txt file */\n";
 }
 
 ##############################################################################
@@ -92,7 +92,7 @@ sub parse_line ($) {
         # Get the major error's name & description
         ($name, $desc) = ($line =~ /^\s*MAJOR,\s*(\w*),\s*(.*)\n/);
 #print "MAJOR: name=$name, desc=$desc\n";
-        
+
         # Check if the name already exists as a major or minor error message
         if(exists($major{$name}) || exists($minor{$name})) {
             die "duplicated name: $name";
@@ -103,12 +103,12 @@ sub parse_line ($) {
     }
     # Parse minor error lines
     elsif($line =~ /^\s*MINOR,/) {
-        my $min_section;           # Minor errors have a section they below to also
+        my $min_section;           # Minor errors have a section they belong to also
 
         # Get the minor error's section, name & description
         ($min_section, $name, $desc) = ($line =~ /^\s*MINOR,\s*(\w*),\s*(\w*),\s*(.*)\n/);
 #print "MINOR: min_section=$min_section, name=$name, desc=$desc\n";
-        
+
         # Check for valid section
         if(!exists($section{$min_section})) {
             die "unknown section: $min_section";
@@ -121,7 +121,7 @@ sub parse_line ($) {
 
         # Store the minor errors in a hash table, indexed by the name
         $minor{$name}=$desc;
-        
+
         # Add the minor error to the list for the section
         push @{$section_list{$min_section}}, $name;
     }
@@ -130,7 +130,7 @@ sub parse_line ($) {
         # Get the section's name & description
         ($name, $desc) = ($line =~ /^\s*SECTION,\s*(\w*),\s*(.*)\n/);
 #print "SECTION: name=$name, desc=$desc\n";
-        
+
         # Check if the section has already been defined
         if(exists($section{$name})) {
             die "duplicated name: $name";
@@ -178,25 +178,25 @@ sub create_public ($) {
     print HEADER "\n/*********************/\n";
     print HEADER   "/* Major error codes */\n";
     print HEADER   "/*********************/\n\n";
-    foreach $name (keys %major) {
+    foreach $name (sort keys %major) {
         printf HEADER "#define %-20s (H5OPEN %s_g)\n",$name,$name;
     }
-    foreach $name (keys %major) {
+    foreach $name (sort keys %major) {
         printf HEADER "H5_DLLVAR hid_t %-20s /* %s */\n","${name}_g;",$major{$name};
     }
 
     # Iterate over all the minor error sections
     print HEADER "\n/*********************/\n";
     print HEADER   "/* Minor error codes */\n";
     print HEADER   "/*********************/\n";
-    while ( ($sect_name, $sect_desc) = each (%section)) {
-        print HEADER "\n/* $sect_desc */\n";
+    foreach $sect_name (sort keys %section) {
+        print HEADER "\n/* $section{$sect_name} */\n";
 
         # Iterate over all the minor errors in each section
-        for $name ( @{$section_list{$sect_name}}) {
+        for $name (sort @{$section_list{$sect_name}}) {
             printf HEADER "#define %-20s (H5OPEN %s_g)\n",$name,$name;
         }
-        for $name ( @{$section_list{$sect_name}}) {
+        for $name (sort @{$section_list{$sect_name}}) {
             printf HEADER "H5_DLLVAR hid_t %-20s /* %s */\n","${name}_g;",$minor{$name};
         }
     }
@@ -213,13 +213,92 @@ sub create_public ($) {
     close HEADER;
 }
 
+##############################################################################
+# Create the generated portion of the H5E major message definition code
+#
+sub create_majdef ($) {
+    my $prefix = shift;         # Get the prefix for the generated file
+    my $file = "H5Emajdef.h";   # Name of file to generate
+    my $name;                   # Name of error message
+
+    # Rename previous file
+#    rename "${prefix}${file}", "${prefix}${file}~" or die "unable to make backup";
+
+    # Open new header file
+    open HEADER, ">${prefix}${file}" or die "unable to modify source";
+
+    # Create file contents
+
+    print_copyright(*HEADER);
+    print_warning(*HEADER);
+    print_startprotect(*HEADER, $file);
+
+    # Iterate over all the major errors
+    print HEADER "\n/***********************************/\n";
+    print HEADER   "/* Major error message definitions */\n";
+    print HEADER   "/***********************************/\n\n";
+    print HEADER   "/* clang-format off */\n";
+    foreach $name (sort keys %major) {
+        printf HEADER "static const H5E_msg_t ${name}_msg_s = {false, \"${major{$name}}\", H5E_MAJOR, &H5E_err_cls_s};\n";
+    }
+    print HEADER   "/* clang-format on */\n";
+
+    print_endprotect(*HEADER, $file);
+
+    # Close header file
+    close HEADER;
+}
+
+##############################################################################
+# Create the generated portion of the H5E minor message definition code
+#
+sub create_mindef ($) {
+    my $prefix = shift;         # Get the prefix for the generated file
+    my $file = "H5Emindef.h";     # Name of file to generate
+    my $name;                   # Name of error message
+    my $sect_name;              # Section of minor error messages
+
+    # Rename previous file
+#    rename "${prefix}${file}", "${prefix}${file}~" or die "unable to make backup";
+
+    # Open new header file
+    open HEADER, ">${prefix}${file}" or die "unable to modify source";
+
+    # Create file contents
+
+    print_copyright(*HEADER);
+    print_warning(*HEADER);
+    print_startprotect(*HEADER, $file);
+
+    # Iterate over all the minor error sections
+    print HEADER "\n/***********************************/\n";
+    print HEADER   "/* Minor error message definitions */\n";
+    print HEADER   "/***********************************/\n\n";
+    print HEADER   "/* clang-format off */\n";
+    foreach $sect_name (sort keys %section) {
+        print HEADER "\n/* $sect_name: $section{$sect_name} */\n";
+
+        # Iterate over all the minor errors in each section
+        for $name (sort @{$section_list{$sect_name}}) {
+            printf HEADER "static const H5E_msg_t ${name}_msg_s = {false, \"${minor{$name}}\", H5E_MINOR, &H5E_err_cls_s};\n";
+        }
+    }
+    print HEADER   "/* clang-format on */\n";
+
+    print_endprotect(*HEADER, $file);
+
+    # Close header file
+    close HEADER;
+}
+
 ##############################################################################
 # Create the generated portion of the H5E initialization code
 #
 sub create_init ($) {
     my $prefix = shift;         # Get the prefix for the generated file
     my $file = "H5Einit.h";     # Name of file to generate
     my $name;                   # Name of error message
+    my $last_name;              # Name of previous error message
     my $desc;                   # Description of error message
     my $sect_name;              # Section of minor error messages
     my $sect_desc;              # Description of section
@@ -242,18 +321,17 @@ sub create_init ($) {
     print HEADER "\n/*********************/\n";
     print HEADER   "/* Major error codes */\n";
     print HEADER   "/*********************/\n\n";
-    foreach $name (keys %major) {
-        print HEADER " "x(0*$indent),"assert(${name}_g==H5I_INVALID_HID);\n";
-        print HEADER " "x(0*$indent),"if((msg = H5E__create_msg(cls, H5E_MAJOR, \"${major{$name}}\"))==NULL)\n";
-        print HEADER " "x(1*$indent),"HGOTO_ERROR(H5E_ERROR, H5E_CANTINIT, FAIL, \"error message initialization failed\");\n";
-        print HEADER " "x(0*$indent),"if((${name}_g = H5I_register(H5I_ERROR_MSG, msg, false))<0)\n";
+    foreach $name (sort keys %major) {
+        print HEADER "/* $name */\n";
+        print HEADER " "x(0*$indent),"assert(H5I_INVALID_HID == ${name}_g);\n";
+        print HEADER " "x(0*$indent),"if((${name}_g = H5I_register(H5I_ERROR_MSG, &${name}_msg_s, false)) < 0)\n";
         print HEADER " "x(1*$indent),"HGOTO_ERROR(H5E_ERROR, H5E_CANTREGISTER, FAIL, \"can't register error message\");\n";
         if ($first_major == 0) {
             print HEADER " "x(0*$indent),"\n/* Remember first major error code ID */\n";
             print HEADER " "x(0*$indent),"assert(H5E_first_maj_id_g==H5I_INVALID_HID);\n";
             print HEADER " "x(0*$indent),"H5E_first_maj_id_g = ${name}_g;\n\n";
             $first_major = 1;
-    }
+        }
         $last_name = $name;
     }
     print HEADER " "x(0*$indent),"\n/* Remember last major error code ID */\n";
@@ -264,15 +342,14 @@ sub create_init ($) {
     print HEADER "\n/*********************/\n";
     print HEADER   "/* Minor error codes */\n";
     print HEADER   "/*********************/\n\n";
-    while ( ($sect_name, $sect_desc) = each (%section)) {
-        print HEADER "\n"," "x(0*$indent),"/* $sect_desc */\n";
+    foreach $sect_name (sort keys %section) {
+        print HEADER "\n/* $section{$sect_name} */\n";
 
         # Iterate over all the minor errors in each section
-        for $name ( @{$section_list{$sect_name}}) {
-            print HEADER " "x(0*$indent),"assert(${name}_g==H5I_INVALID_HID);\n";
-            print HEADER " "x(0*$indent),"if((msg = H5E__create_msg(cls, H5E_MINOR, \"${minor{$name}}\"))==NULL)\n";
-            print HEADER " "x(1*$indent),"HGOTO_ERROR(H5E_ERROR, H5E_CANTINIT, FAIL, \"error message initialization failed\");\n";
-            print HEADER " "x(0*$indent),"if((${name}_g = H5I_register(H5I_ERROR_MSG, msg, true))<0)\n";
+        for $name (sort @{$section_list{$sect_name}}) {
+            print HEADER "/* $name */\n";
+            print HEADER " "x(0*$indent),"assert(H5I_INVALID_HID == ${name}_g);\n";
+            print HEADER " "x(0*$indent),"if((${name}_g = H5I_register(H5I_ERROR_MSG, &${name}_msg_s, false)) < 0)\n";
             print HEADER " "x(1*$indent),"HGOTO_ERROR(H5E_ERROR, H5E_CANTREGISTER, FAIL, \"can't register error message\");\n";
 
             if ($first_minor == 0) {
@@ -319,25 +396,25 @@ sub create_term ($) {
 
     # Iterate over all the major errors
     print HEADER "\n/* Reset major error IDs */\n";
-    foreach $name (keys %major) {
-        print HEADER " "x($indent),"\n${name}_g=";
+    foreach $name (sort keys %major) {
+        print HEADER " "x($indent),"${name}_g =\n";
     }
-    print HEADER " H5I_INVALID_HID;\n";
-    print HEADER " "x(0*$indent),"H5E_first_maj_id_g = H5I_INVALID_HID;\n\n";
+    print HEADER " "x(2*$indent),"H5I_INVALID_HID;\n";
+    print HEADER "\n"," "x(0*$indent),"H5E_first_maj_id_g = H5I_INVALID_HID;\n";
     print HEADER " "x(0*$indent),"H5E_last_maj_id_g = H5I_INVALID_HID;\n\n";
 
     # Iterate over all the minor error sections
     print HEADER "\n/* Reset minor error IDs */\n";
-    while ( ($sect_name, $sect_desc) = each (%section)) {
-        print HEADER "\n"," "x(0*$indent),"\n/* $sect_desc */";
+    foreach $sect_name (sort keys %section) {
+        print HEADER "\n/* $sect_name: $section{$sect_name} */\n";
 
         # Iterate over all the minor errors in each section
-        for $name ( @{$section_list{$sect_name}}) {
-            print HEADER " "x($indent),"\n${name}_g=";
+        for $name (sort @{$section_list{$sect_name}}) {
+            print HEADER " "x($indent),"${name}_g =\n";
         }
     }
-    print HEADER " H5I_INVALID_HID;\n";
-    print HEADER " "x(0*$indent),"H5E_first_min_id_g = H5I_INVALID_HID;\n\n";
+    print HEADER " "x(2*$indent),"H5I_INVALID_HID;\n";
+    print HEADER "\n"," "x(0*$indent),"H5E_first_min_id_g = H5I_INVALID_HID;\n";
     print HEADER " "x(0*$indent),"H5E_last_min_id_g = H5I_INVALID_HID;\n\n";
 
     print_endprotect(*HEADER, $file);
@@ -356,6 +433,7 @@ sub create_define ($) {
     my $desc;                   # Description of error message
     my $sect_name;              # Section of minor error messages
     my $sect_desc;              # Description of section
+    my $num_msg;                # Number of messages
 
     # Rename previous file
 #    rename "${prefix}${file}", "${prefix}${file}~" or die "unable to make backup";
@@ -370,21 +448,29 @@ sub create_define ($) {
     print_startprotect(*HEADER, $file);
 
     # Iterate over all the major errors
+    $num_msg = 0;
     print HEADER "\n/* Major error IDs */\n";
-    foreach $name (keys %major) {
-        printf HEADER "hid_t %-20s = FAIL;      /* %s */\n","${name}_g",$major{$name};
+    foreach $name (sort keys %major) {
+        printf HEADER "hid_t %-20s = H5I_INVALID_HID;      /* %s */\n","${name}_g",$major{$name};
+        $num_msg++;
     }
+    print HEADER "\n/* Number of major error messages */\n";
+    printf HEADER "#define H5E_NUM_MAJ_ERRORS %d\n", $num_msg;
 
     # Iterate over all the minor error sections
+    $num_msg = 0;
     print HEADER "\n/* Minor error IDs */\n";
-    while ( ($sect_name, $sect_desc) = each (%section)) {
-        print HEADER "\n/* $sect_desc */\n";
+    foreach $sect_name (sort keys %section) {
+        print HEADER "\n/* $sect_name: $section{$sect_name} */\n";
 
         # Iterate over all the minor errors in each section
-        for $name ( @{$section_list{$sect_name}}) {
-            printf HEADER "hid_t %-20s = FAIL;      /* %s */\n","${name}_g",$minor{$name};
+        for $name (sort @{$section_list{$sect_name}}) {
+            printf HEADER "hid_t %-20s = H5I_INVALID_HID;      /* %s */\n","${name}_g",$minor{$name};
+            $num_msg++;
         }
     }
+    print HEADER "\n/* Number of minor error messages */\n";
+    printf HEADER "#define H5E_NUM_MIN_ERRORS %d\n", $num_msg;
 
     print_endprotect(*HEADER, $file);
 
@@ -409,10 +495,14 @@ for $file (@ARGV) {
         }
     }
     close SOURCE;
-  
+
     # Create header files
     print "Generating 'H5Epubgen.h'\n";
     create_public($prefix);
+    print "Generating 'H5Emajdef.h'\n";
+    create_majdef($prefix);
+    print "Generating 'H5Emindef.h'\n";
+    create_mindef($prefix);
     print "Generating 'H5Einit.h'\n";
     create_init($prefix);
     print "Generating 'H5Eterm.h'\n";

diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
@@ -870,6 +870,8 @@ set (H5_PRIVATE_HEADERS
 
     ${HDF5_SRC_DIR}/H5Edefin.h
     ${HDF5_SRC_DIR}/H5Einit.h
+    ${HDF5_SRC_DIR}/H5Emajdef.h
+    ${HDF5_SRC_DIR}/H5Emindef.h
     ${HDF5_SRC_DIR}/H5Epkg.h
     ${HDF5_SRC_DIR}/H5Eprivate.h
     ${HDF5_SRC_DIR}/H5Eterm.h

diff --git a/src/H5D.c b/src/H5D.c
@@ -2137,7 +2137,7 @@ H5Dformat_convert(hid_t dset_id)
 
     /* Convert the dataset */
     if (H5VL_dataset_optional(vol_obj, &vol_cb_args, H5P_DATASET_XFER_DEFAULT, H5_REQUEST_NULL) < 0)
-        HGOTO_ERROR(H5E_DATASET, H5E_INTERNAL, FAIL, "can't convert dataset format");
+        HGOTO_ERROR(H5E_DATASET, H5E_CANTUPDATE, FAIL, "can't convert dataset format");
 
 done:
     FUNC_LEAVE_API(ret_value)
@@ -2442,7 +2442,7 @@ H5Dchunk_iter(hid_t dset_id, hid_t dxpl_id, H5D_chunk_iter_op_t op, void *op_dat
 
     /* Iterate over the chunks */
     if ((ret_value = H5VL_dataset_optional(vol_obj, &vol_cb_args, dxpl_id, H5_REQUEST_NULL)) < 0)
-        HERROR(H5E_BADITER, H5E_BADITER, "error iterating over dataset chunks");
+        HERROR(H5E_DATASET, H5E_BADITER, "error iterating over dataset chunks");
 
 done:
     FUNC_LEAVE_API(ret_value)

diff --git a/src/H5Dint.c b/src/H5Dint.c
@@ -1495,7 +1495,7 @@ H5D_open(const H5G_loc_t *loc, hid_t dapl_id)
     /* Check if dataset was already open */
     if (NULL == (shared_fo = (H5D_shared_t *)H5FO_opened(dataset->oloc.file, dataset->oloc.addr))) {
         /* Clear any errors from H5FO_opened() */
-        H5E_clear_stack(NULL);
+        H5E_clear_stack();
 
         /* Open the dataset object */
         if (H5D__open_oid(dataset, dapl_id) < 0)

diff --git a/src/H5Dvirtual.c b/src/H5Dvirtual.c
@@ -884,7 +884,7 @@ H5D__virtual_open_source_dset(const H5D_t *vdset, H5O_storage_virtual_ent_t *vir
             src_file_open = true;
         else
             /* Reset the error stack */
-            H5E_clear_stack(NULL);
+            H5E_clear_stack();
     } /* end if */
     else
         /* Source file is ".", use the virtual dataset's file */
@@ -906,7 +906,7 @@ H5D__virtual_open_source_dset(const H5D_t *vdset, H5O_storage_virtual_ent_t *vir
         /* Dataset does not exist */
         if (NULL == source_dset->dset) {
             /* Reset the error stack */
-            H5E_clear_stack(NULL);
+            H5E_clear_stack();
 
             source_dset->dset_exists = false;
         } /* end if */