lkml.org 
[lkml]   [2016]   [Jul]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH 32/32] ver_linux: 'printversion()' function definition
On Tue, Jun 28, 2016 at 5:48 PM,  <Valdis.Kletnieks@vt.edu> wrote:
> 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....
>

I do not mind reworking the script to make it report errors to the
user, if that is the behaviour expected by the community. The proposed
implementation follows the behaviour of the previous iterations of the
script based on its development history, which is to be silent about
those kinds of errors.

I have just been waiting for more feedback from the community before
making any further modifications to the proposed implementation.

Thanks.

\
 
 \ /
  Last update: 2016-07-02 10:01    [W:0.083 / U:2.676 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site