google / szl

A compiler and runtime for the Sawzall language
Other
69 stars 16 forks source link

[PATCH] --noboolflag=false incorrectly parsed as --noboolflag #14

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
ProcessCommandLineArguments (in utilities/commandlineflags.cc) recognizes 
"--no<flagname>" by comparing the known flagname with p[2..len+2], but it then 
incorrectly goes on to compare "p[len] == '='" in the rest of the function as a 
way of distinguishing "--flag=value" from "--flag value".

This goes wrong when the flag is boolean, "no" is present, and the "=value" is 
also present.  The handling of "false" and "true" for bool flags is intended to 
be inverted when "no" is present.  Unfortunately, the presence of "no" causes 
the code to fail to find the '=', and so it proceeds as if the "=value" was 
absent.  When the value is "true" this works out, but not when it's "false".

The patch adds 2 to len when the flag was recognized as "no" + a bool flag (and 
not, say, a bool flag happening to begin with no, such as "--notify").

Original issue reported on code.google.com by aecolley on 13 Nov 2010 at 2:04

Attachments:

GoogleCodeExporter commented 9 years ago
This follows the behavior of the Google gflags library 
(http://code.google.com/p/google-gflags).  In the gflags documentation, 
--boolflag=true/false, --boolflag or --noboolflag are the accepted variants.  
--noboolflag=value is not a valid variant, so the value is ignored when you 
specify the "no" form. 

Original comment by dbh@google.com on 13 Nov 2010 at 8:41

GoogleCodeExporter commented 9 years ago
Upon further inspection, it looks like the fact that lines 126 and 128 use
!no for the value of bool typed flags gives the impression that 
--noboolflag=value might work.  The fact that it does not work is correct, 
as the semantics are supposed to be like gflags, but the code should convey 
the intent.  This is a lightweight rewrite of a subset of gflags 
functionality and I'm not familiar with the code, but looking through I 
see a few other potential points of divergence (e.g. gflags allows "1", 
"t", "true", "y", "yes" for true and "0", "f", "false", "n", "no" for
false when you specify --boolflag=value).  Fixing these is not high 
priority, though.

Original comment by dbh@google.com on 14 Nov 2010 at 1:06

GoogleCodeExporter commented 9 years ago
Fair enough.

Original comment by aecolley on 14 Nov 2010 at 4:54