tuxji commented on a change in pull request #525: URL: https://github.com/apache/daffodil/pull/525#discussion_r610815704
########## File path: daffodil-runtime2/src/main/resources/c/libruntime/errors.h ########## @@ -0,0 +1,117 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#ifndef ERRORS_H +#define ERRORS_H + +#include <stdio.h> // for FILE, size_t +#include <stdint.h> // for int64_t + +// ErrorCode - types of errors which could occur + +enum ErrorCode +{ + ERR_CHOICE_KEY, + ERR_FILE_CLOSE, + ERR_FILE_FLUSH, + ERR_FILE_OPEN, + ERR_FIXED_VALUE, + ERR_INFOSET_READ, + ERR_INFOSET_WRITE, + ERR_PARSE_BOOL, + ERR_STACK_EMPTY, + ERR_STACK_OVERFLOW, + ERR_STACK_UNDERFLOW, + ERR_STREAM_EOF, + ERR_STREAM_ERROR, + ERR_STRTOBOOL, + ERR_STRTOD_ERRNO, + ERR_STRTOI_ERRNO, + ERR_STRTONUM_EMPTY, + ERR_STRTONUM_NOT, + ERR_STRTONUM_RANGE, + ERR_XML_DECL, + ERR_XML_ELEMENT, + ERR_XML_ERD, + ERR_XML_GONE, + ERR_XML_INPUT, + ERR_XML_LEFT, + ERR_XML_MISMATCH, + ERR_XML_WRITE +}; + +// Error - specific error occuring now + +typedef struct Error +{ + enum ErrorCode code; + union + { + const char *s; // for %s Review comment: Oh, I didn't consider passing schema filename/lineno information for a diagnostic. We'd still want to pass a string or int64 for some errors, so I would store an ERD (or a Location object) in its own separate Error member field, not as an alternative field in the union. We currently don't store any schema filename/lineno in the ERD metadata, so I'll add this idea to the runtime2 todos for a later time. We also might want to pass a PState/UState for some errors to convey filename/position information about where the error was detected in the data, yes? ########## File path: daffodil-runtime2/src/main/resources/c/libruntime/parsers.c ########## @@ -142,36 +136,38 @@ define_parse_endian_integer(le, uint, 32) define_parse_endian_integer(le, uint, 64) define_parse_endian_integer(le, uint, 8) -// Define function to parse fill bytes until end position is reached +// Parse fill bytes until end position is reached void parse_fill_bytes(size_t end_position, PState *pstate) { - while (!pstate->error_msg && pstate->position < end_position) + union { - char buffer; - size_t count = fread(&buffer, 1, sizeof(buffer), pstate->stream); + char c_val[1]; + } buffer; - pstate->position += count; - if (count < sizeof(buffer)) - { - pstate->error_msg = eof_or_error_msg(pstate->stream); - } + while (pstate->position < end_position) + { + read_stream_update_position; } } -// Define function to validate number is same as fixed value after parse +// Validate parsed number is same as fixed value void parse_validate_fixed(bool same, const char *element, PState *pstate) { - UNUSED(element); // because managing strings hard in embedded C - if (!pstate->error_msg && !same) + if (!same) { - // Error message would be easier to assemble and - // internationalize if we used an error struct with multiple - // fields instead of a const char string. - pstate->error_msg = "Parse: Value of element does not match value of " - "its 'fixed' attribute"; + Diagnostics *validati = need_diagnostics(); + pstate->validati = validati; + + if (validati->length < + sizeof(validati->array) / sizeof(*validati->array)) + { + Error *error = &validati->array[validati->length++]; + error->code = ERR_FIXED_VALUE; + error->s = element; + } Review comment: This is the compile-time array length calculation I was talking about. I'll replace this if statement with an add_diagnostic call as you suggested. Do you really want add_diagnostic to stop the program as soon as the array fills up, or simply stop adding any more diagnostics to the array? ```c const Error error = {ERR_FIXED_VALUE, {element}}; add_diagnostic(validati, &error); ``` ########## File path: daffodil-runtime2/src/main/resources/c/libruntime/errors.h ########## @@ -0,0 +1,117 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#ifndef ERRORS_H +#define ERRORS_H + +#include <stdio.h> // for FILE, size_t +#include <stdint.h> // for int64_t + +// ErrorCode - types of errors which could occur + +enum ErrorCode +{ + ERR_CHOICE_KEY, + ERR_FILE_CLOSE, + ERR_FILE_FLUSH, + ERR_FILE_OPEN, + ERR_FIXED_VALUE, + ERR_INFOSET_READ, + ERR_INFOSET_WRITE, + ERR_PARSE_BOOL, + ERR_STACK_EMPTY, + ERR_STACK_OVERFLOW, + ERR_STACK_UNDERFLOW, + ERR_STREAM_EOF, + ERR_STREAM_ERROR, + ERR_STRTOBOOL, + ERR_STRTOD_ERRNO, + ERR_STRTOI_ERRNO, + ERR_STRTONUM_EMPTY, + ERR_STRTONUM_NOT, + ERR_STRTONUM_RANGE, + ERR_XML_DECL, + ERR_XML_ELEMENT, + ERR_XML_ERD, + ERR_XML_GONE, + ERR_XML_INPUT, + ERR_XML_LEFT, + ERR_XML_MISMATCH, + ERR_XML_WRITE +}; + +// Error - specific error occuring now + +typedef struct Error +{ + enum ErrorCode code; + union + { + const char *s; // for %s + int64_t d64; // for %d64 + }; +} Error; + +// Diagnostics - array of validation errors + +typedef struct Diagnostics +{ + Error array[100]; Review comment: Actually, the C standard allows us to find an array's size at compile time using two sizeof calls: `sizeof(array) / sizeof(*array)`. Since C doesn't allow us to define a scoped compile-time constant inside a header file easily (we'd have to define a macro or an enumeration, both of which go into the global namespace), I thought it was better to use 100 in the header file and two sizeof calls to get the array's size. However, I like your idea of defining an add_diagnostic abstraction so I'll define that function in errors.[ch] and call it in parsers.c & unparsers.c. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected]
