On Fri, 9 Dec 2022 12:56:08 GMT, Julian Waters <jwat...@openjdk.org> wrote:
>> make/autoconf/jdk-version.m4 line 94: >> >>> 92: properties of MS Windows binaries.], >>> 93: DEFAULT_DESC: [not specified], >>> 94: CHECK_VALUE: [ >> >> I see that this check is repeated all over the place. I suggest that you >> create a new `TYPE` for this. I suggest calling it `identifier`. Then you >> replace `TYPE: string` with `TYPE: identifier` in all places where we check >> that the value is non-empty and contains only printable characters. The type >> itself is declared in line 692 in util.m4 (and documented on line 582). Then >> you need to add a type checking method: >> >> AC_DEFUN([UTIL_CHECK_TYPE_identifier], >> [ >> # Check that the argument is non-empty and only contains printable >> characters >> if test "x$1" = x; then >> FAILURE="Value cannot be empty" >> elif [ ! [[ $1 =~ ^[[:print:]]*$ ]] ]; then >> FAILURE="Value contains non-printing characters: $1" >> fi >> ]) > > Seems to me like a util function for CHECK_VALUE would be a slightly better > approach than creating a new type, like the one we already have: > > > ############################################################################### > # Helper functions for CHECK_VALUE in ARG_WITH. > AC_DEFUN([UTIL_CHECK_STRING_NON_EMPTY], > [ > if test "x$RESULT" = "x"; then > FAILURE="Value cannot be empty" > fi > ]) Oh, I forgot about that one. Yes, a `UTIL_CHECK_STRING_PRINTABLE` or something like that would be an alternative approach. I'm not sure it's better than creating a separate type, but it's not worse either, so if you prefer it, do it. Just make sure it is a single function that tests both non-emptiness and correct character range and that the name of the function is not longer than the code. ;-) ------------- PR: https://git.openjdk.org/jdk/pull/11020
