From 85611f3f4a5c861e981f317a091fbba230e1a0a8 Mon Sep 17 00:00:00 2001 From: uramirez8707 <49168881+uramirez8707@users.noreply.github.com> Date: Thu, 1 Aug 2024 11:49:53 -0400 Subject: [PATCH] fix: yaml parser error checking (#1563) --- parser/yaml_parser.F90 | 33 +++++++++++++++++++++---- parser/yaml_parser_binding.c | 12 ++++++---- test_fms/parser/parser_demo.F90 | 2 +- test_fms/parser/parser_demo2.F90 | 2 +- test_fms/parser/test_yaml_parser.sh | 37 ++++++++++++++++++++++++++++- 5 files changed, 73 insertions(+), 13 deletions(-) diff --git a/parser/yaml_parser.F90 b/parser/yaml_parser.F90 index 9c9ae0a793..3a3c7f0051 100644 --- a/parser/yaml_parser.F90 +++ b/parser/yaml_parser.F90 @@ -59,6 +59,12 @@ module yaml_parser_mod module procedure get_value_from_key_1d end interface get_value_from_key +!! Error codes from open_and_parse_file_wrap +integer, parameter :: MISSING_FILE = -1 !< Error code if the yaml file is missing +integer, parameter :: PARSER_INIT_ERROR = -2 !< Error code if unable to create a parser object +integer, parameter :: INVALID_YAML = -3 !< Error code if unable to parse a yaml file +integer, parameter :: SUCCESSFUL = 1 !< "Error" code if the parsing was successful + !> @brief c functions binding !> @ingroup yaml_parser_mod interface @@ -66,11 +72,11 @@ module yaml_parser_mod !> @brief Private c function that opens and parses a yaml file (see yaml_parser_binding.c) !! @return Flag indicating if the read was successful function open_and_parse_file_wrap(filename, file_id) bind(c) & - result(success) + result(error_code) use iso_c_binding, only: c_char, c_int, c_bool character(kind=c_char), intent(in) :: filename(*) !< Filename of the yaml file integer(kind=c_int), intent(out) :: file_id !< File id corresponding to the yaml file that was opened - logical(kind=c_bool) :: success !< Flag indicating if the read was successful + logical(kind=c_int) :: error_code !< Flag indicating the error message (1 if sucessful) end function open_and_parse_file_wrap !> @brief Private c function that checks if a file_id is valid (see yaml_parser_binding.c) @@ -240,7 +246,7 @@ function open_and_parse_file(filename) & result(file_id) character(len=*), intent(in) :: filename !< Filename of the yaml file - logical :: success !< Flag indicating if the read was successful + integer :: error_code !< Flag indicating any errors in the parsing or 1 if sucessful logical :: yaml_exists !< Flag indicating whether the yaml exists integer :: file_id @@ -251,11 +257,28 @@ function open_and_parse_file(filename) & call mpp_error(NOTE, "The yaml file:"//trim(filename)//" does not exist, hopefully this is your intent!") return end if - success = open_and_parse_file_wrap(trim(filename)//c_null_char, file_id) - if (.not. success) call mpp_error(FATAL, "Error opening the yaml file:"//trim(filename)//". Check the file!") + error_code = open_and_parse_file_wrap(trim(filename)//c_null_char, file_id) + call check_error_code(error_code, filename) end function open_and_parse_file +!> @brief Checks the error code from a open_and_parse_file_wrap function call +subroutine check_error_code(error_code, filename) + integer, intent(in) :: error_code + character(len=*), intent(in) :: filename + + select case (error_code) + case (SUCCESSFUL) + return + case (MISSING_FILE) + call mpp_error(FATAL, "Error opening the yaml file:"//trim(filename)) + case (PARSER_INIT_ERROR) + call mpp_error(FATAL, "Error initializing the parser for the file:"//trim(filename)) + case (INVALID_YAML) + call mpp_error(FATAL, "Error parsing the file:"//trim(filename)//". Check that your yaml file is valid") + end select +end subroutine check_error_code + !> @brief Gets the key from a file id subroutine get_key_name(file_id, key_id, key_name) integer, intent(in) :: key_id !< Id of the key-value pair of interest diff --git a/parser/yaml_parser_binding.c b/parser/yaml_parser_binding.c index 42795fbba8..778b6267dc 100644 --- a/parser/yaml_parser_binding.c +++ b/parser/yaml_parser_binding.c @@ -300,7 +300,7 @@ bool is_valid_file_id(int *file_id) /* @brief Private c function that opens and parses a yaml file and saves it in a struct @return Flag indicating if the read was sucessful */ -bool open_and_parse_file_wrap(char *filename, int *file_id) +int open_and_parse_file_wrap(char *filename, int *file_id) { yaml_parser_t parser; yaml_token_t token; @@ -330,9 +330,9 @@ bool open_and_parse_file_wrap(char *filename, int *file_id) /* printf("Opening file: %s.\nThere are %i files opened.\n", filename, j); */ file = fopen(filename, "r"); - if (file == NULL) return false; + if (file == NULL) return -1; - if(!yaml_parser_initialize(&parser)) return false; + if(!yaml_parser_initialize(&parser)) return -2; my_files.files[j].keys = (key_value_pairs*)calloc(1, sizeof(key_value_pairs)); @@ -341,7 +341,9 @@ bool open_and_parse_file_wrap(char *filename, int *file_id) /* Set input file */ yaml_parser_set_input_file(&parser, file); do { - yaml_parser_scan(&parser, &token); + if (!yaml_parser_scan(&parser, &token)) { + return -3; + } switch(token.type) { case YAML_KEY_TOKEN: @@ -420,7 +422,7 @@ bool open_and_parse_file_wrap(char *filename, int *file_id) /* printf("closing file: %s\n", filename); */ fclose(file); - return true; + return 1; } #endif diff --git a/test_fms/parser/parser_demo.F90 b/test_fms/parser/parser_demo.F90 index 5b4ccfd88e..208e41e807 100644 --- a/test_fms/parser/parser_demo.F90 +++ b/test_fms/parser/parser_demo.F90 @@ -38,7 +38,6 @@ program parser_demo real(kind=r8_kind) :: r8_buffer !< Buffer to read r8 to call fms_init -call fms_end diag_yaml_id = open_and_parse_file("diag_table.yaml") print *, "" @@ -113,6 +112,7 @@ program parser_demo print *, "" enddo deallocate(file_ids) +call fms_end #endif end program parser_demo diff --git a/test_fms/parser/parser_demo2.F90 b/test_fms/parser/parser_demo2.F90 index c230559a4e..674ab85fbb 100644 --- a/test_fms/parser/parser_demo2.F90 +++ b/test_fms/parser/parser_demo2.F90 @@ -39,7 +39,6 @@ program parser_demo character(len=255) :: key_name !< The name of a key call fms_init -call fms_end diag_yaml_id = open_and_parse_file("diag_table.yaml") print *, "" @@ -102,6 +101,7 @@ program parser_demo print *, "" enddo deallocate(file_ids) +call fms_end #endif diff --git a/test_fms/parser/test_yaml_parser.sh b/test_fms/parser/test_yaml_parser.sh index 80c386e687..e5405baaf3 100755 --- a/test_fms/parser/test_yaml_parser.sh +++ b/test_fms/parser/test_yaml_parser.sh @@ -26,7 +26,7 @@ . ../test-lib.sh if [ ! -z $parser_skip ]; then - SKIP_TESTS='test_yaml_parser.[1-23]' + SKIP_TESTS='test_yaml_parser.[1-25]' fi touch input.nml @@ -294,4 +294,39 @@ _EOF test_expect_success "Generic blocks names" ' mpirun -n 1 ./generic_blocks ' + +cat <<_EOF > diag_table.yaml +title: c384L49_esm5PIcontrol +baseDate: [1960 1 1 1 1 1 1] +diag_files: +- fileName: "atmos_daily" + freq: 24 + frequnit: hours + timeunit: days + unlimdim: time + varlist: + - varName: tdata + reduction: False + module: mullions + mullions: 10 + fill_value: -999.9 + - varName: pdata + outName:pressure + reduction: False + kind: double + module: "moist" +- fileName: atmos_8xdaily + freq: 3 + frequnit: hours + timeunit: days + unlimdim: time + varlist: + - varName: tdata + reduction: False + module: "moist" +_EOF + +test_expect_failure "Use an invalid yaml" ' + mpirun -n 1 ./parser_demo +' test_done