Re: [PATCH 32/32] ver_linux: 'printversion()' function definition

[Valdis.Kletnieks@vt.edu]

[-- Attachment #1: Type: text/plain, Size: 1919 bytes --]

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....


[-- Attachment #2: Type: application/pgp-signature, Size: 848 bytes --]