From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752203AbcF1Ost (ORCPT ); Tue, 28 Jun 2016 10:48:49 -0400 Received: from outbound.smtp.vt.edu ([198.82.183.121]:45250 "EHLO omr1.cc.vt.edu" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751068AbcF1Osr (ORCPT ); Tue, 28 Jun 2016 10:48:47 -0400 X-Mailer: exmh version 2.8.0 04/21/2012 with nmh-1.6+dev To: Alexander Kapshuk Cc: linux-kernel , Greg KH Subject: Re: [PATCH 32/32] ver_linux: 'printversion()' function definition From: Valdis.Kletnieks@vt.edu In-Reply-To: References: <1467109146-20331-1-git-send-email-alexander.kapshuk@gmail.com> <1467109146-20331-32-git-send-email-alexander.kapshuk@gmail.com> <54584.1467118075@turing-police.cc.vt.edu> Mime-Version: 1.0 Content-Type: multipart/signed; boundary="==_Exmh_1467125319_1978P"; micalg=pgp-sha1; protocol="application/pgp-signature" Content-Transfer-Encoding: 7bit Date: Tue, 28 Jun 2016 10:48:39 -0400 Message-ID: <3402.1467125319@turing-police.cc.vt.edu> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --==_Exmh_1467125319_1978P Content-Type: text/plain; charset=us-ascii 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.... --==_Exmh_1467125319_1978P Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 Comment: Exmh version 2.5 07/13/2001 iQIVAwUBV3KORwdmEQWDXROgAQJMexAApbfgfTOEh+gKaiN321VD1hVJRO4RHOSb a9cmIc5hRtjUR32H8HudSYg5uRQhV+NTeDP+EUUbsGJgKUqM4WvxGdo0yHn85+sc BwW7cWO6sWm1y3WoW7RCh1mO28t4sWRHbr3ewPS7Y3eKAOTXH5EnPxjqhakDw568 WBn8JsgHzM5RhhqbQUxot6d00zwGOe9BkpiXVDC8B9lHEgHTbbJw1qBev3X0ILDK QYwOpbRbkeIIepQrYTORKW6+vRPIbj44LY7feXqW2Lr0+HMf3qS/9fia890O4A0T czrXhUJ9hWwmunCMHHzRxJqrGPp+msEK0lKTmaB75U3OFozSYgTAta0TRudOfvqo 7IrYhO8EY4qeY1T7aTnBbx7dhOVrcndznmsiaWOCUAIEnQIgb2v6jel7FljUQySj 4b9rQe/zejo7u8IXCRwqpt5pxDM5VyOv2M7rJVhxq77dkDKRPMefrmD5fDhI4ZPa ap9w0R7DgTFh8Mbb7FdyVEatRoQs5DsCu9tBno1IvaGAmvoo+h9hJK2/6V7BnAwQ Sv2+qshTEgNhJ86tLCmf7BLv1TjqlQ3wxr/+p8WOA/aTD0P+mJ0oYYG/FecTXtMz ADekk4QlyR0TSVsJriKpHppJ3d2RQ+i8ONuJIKepKSNYcU1dnQzZE/ha8suhfrPc z9zFj+FALLc= =Dn4I -----END PGP SIGNATURE----- --==_Exmh_1467125319_1978P--