Bug 10590 - perl-MDK-Common: text2bool should handle "1" also
Summary: perl-MDK-Common: text2bool should handle "1" also
Status: RESOLVED WONTFIX
Alias: None
Product: Mageia
Classification: Unclassified
Component: RPM Packages (show other bugs)
Version: Cauldron
Hardware: All Linux
Priority: Normal normal
Target Milestone: ---
Assignee: Mageia Bug Squad
QA Contact:
URL:
Whiteboard:
Keywords: PATCH
Depends on:
Blocks: 10269
  Show dependency treegraph
 
Reported: 2013-06-22 13:04 CEST by Pablo Saratxaga
Modified: 2013-07-05 16:10 CEST (History)
4 users (show)

See Also:
Source RPM: perl-MDK-Common-1.2.29-3.mga4.src.rpm
CVE:
Status comment:


Attachments

Description Pablo Saratxaga 2013-06-22 13:04:38 CEST
text2bool is a helper function in our commong perl framework;
it is handy to parse boolean-like shell variables.

However, while it handles correctly FOO=yes and FOO=true, i ignores FOO=1 (considering it as being false).

fix is easy: change

sub text2bool { my $t = lc($_[0]); $t eq "true" || $t eq "yes" ? 1 : 0 }

to:

sub text2bool { my $t = lc($_[0]); $t eq "true" || $t eq "yes" || $t eq "1" ? 1 : 0 }

or maybe:

sub text2bool { $_[0] =~ m/^(true|yes|1)$/i ? 1 : 0 }

(I don't know which one will be the most efficient)


Reproducible: 

Steps to Reproduce:
Pablo Saratxaga 2013-06-22 13:05:02 CEST

CC: (none) => pablo

Pablo Saratxaga 2013-06-22 13:11:09 CEST

Blocks: (none) => 10269

Comment 1 Nicolas Lécureuil 2013-06-22 13:14:24 CEST
olivier, thierry, can you review this patch ?

CC: (none) => mageia, nicolas.lecureuil, thierry.vignaud

Manuel Hiebel 2013-06-22 18:07:55 CEST

Keywords: (none) => PATCH

Comment 2 Thierry Vignaud 2013-06-23 17:23:32 CEST
NACK.
It behaves as intended and as documented:

       to_bool(SCALAR)
           returns a value in { 0, 1 }

       bool2text(SCALAR)
           returns a value in { "true", "false" }

       bool2yesno(SCALAR)
           returns a value in { "yes", "no" }

       text2bool(STRING)
           inverse of "bool2text" and "bool2yesno"

*bool() returns a value in { 0, 1 }
bool2*() takes a value in { 0, 1 } and return a string

Status: NEW => RESOLVED
Resolution: (none) => WONTFIX

Comment 3 Pablo Saratxaga 2013-06-24 00:13:18 CEST
the problem is text2bool will return 0 (false) for things like FOO=1 (which gets hashed as 'FOO' => '1', that is '1' as a string).

In network ifcfg files for example (and I suppose in a lot of shell-parseable config files) there is a mix of FOO=(yes|no) and FOO=(1|0) (or even FOO=(yes|no|1|0); that is the case of most boolean DHCP related config parameters, using "1" on them is valid for ifcfg format, but then parsing with text2bool will return true for "yes" but false for "no" (corect), "0" (correct) and for "1" (incorrect) ).

text2bool is nice to handle those parsing; but it fails for cases where "1" (string "1") can be used as synonym of "yes".

Adding support for the case of "1" (*STRING* "1", not numeric 1) will be a big plus; so that we can use text2bool($ref->{FOO}) and be sure to always have a correct return value.

please reconsider adding that functionality (otherwise we should just remove all use of text2bool in network tools that parse ifcfg files)
Comment 4 Thierry Vignaud 2013-06-24 14:56:37 CEST
We could go for this then:

sub text2bool { member(lc($_[0]), qw(true yes 1)) }
Comment 5 Pablo Saratxaga 2013-06-25 00:21:27 CEST
yes; that would be perfect.
Comment 6 Pablo Saratxaga 2013-07-05 16:10:43 CEST
oh; I just discovered another pseudo-boolean to add to the list = on/off
(it is used by some bridge related ifcfg settings).
So:  

   sub text2bool { member(lc($_[0]), qw(true yes on 1)) }

will handle the couples true/false, yes/no, on/off, '1'/'0'

thanks

Note You need to log in before you can comment on or make changes to this bug.