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

[Alexander Kapshuk]

On Tue, Jun 28, 2016 at 3:47 PM,  <Valdis.Kletnieks@xxxxxx> wrote:
> On Tue, 28 Jun 2016 13:19:06 +0300, Alexander Kapshuk said:
>> Definition of the 'printversion()' function. The function tests whether
>> the variable that contains the version number is empty, and prints
>> the name of the utility and its version number as a formatted string,
>> if the version number is not an empty value.
>
> This needs to be the first patch in the series, not the last, so that if
> you're applying the patches one by one, the result still works, which allows
> incremental testing after each patch.
>
> Putting it last means you have to apply all 32 patches before you get
> something you can test.

Thanks for your response.

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.

>
> The idea is good, however.
>
> One thing that might be good now that it's only one chunk of code, is to add
> some code to check between the following cases: it's something like isdnctrl or
> cardmgr that's not installed because it's truly optional in today's world where
> ISDN or PCMCIA slots have become rare, or if the regexp doing the matching
> failed because the utility is present but produced unexpected output.
>
> Another useful thing would be distinguishing between must-have things
> like the toolchain where the build *will* fail, and optionals that
> are only used in some configurations.  This will probably require reordering
> the output (and corresponding changes to Documentation/Changes)
>
>
>

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.