On Tue, 28 Jun 2016 17:40:36 +0300, Alexander Kapshuk said: > Seeing this is a complete rewrite of the script from the shell > language into awk, one would not be able to apply the patches > submitted incrementally to be able to test each change being > introduced separately. In this respect, defining the 'version()' and > the 'printversion()' functions ahead of the code that calls them makes > no difference. Good point for this instance. Note that reviewers in general expect that sort of "must still work after every incremental commit" structure in the future, though... > If a utility being queried is not available on a given system, the > shell that executes the script outputs an error message along the > lines of 'ver_linux: line number where the call is made: error > message: name_of_utility Not found or something like that. This is > taken care of by the 'if' block in the 'version()' function: > if (!/ver_linux/...) {} > > The second condition that must be met in the 'if' block above takes > care of situations where input does not match the regular expression > for the version number: > if (... && match($0, /[0-9]+([.]?[0-9]+)+/)) {} > > Only when the 'if' block above evaluates as being true is the 'ver' > variable set to the string matched by the regular expression. > The 'printversion()' function will not print anything should the value > representing the version number, a list of kernel modules, etc, be > found empty. > > If I understood your commentary correctly, the proposed implementation > does address the issues you raise. Unless I misread something. I meant that the distinction should be surfaced to the user - if the binary returns a version string that our regexp can't parse, it's probably a complete rewrite and we should *tell* the user that we don't know what's going on. An "if result = empty then print '(unable to identify version)'" should be sufficient....