[Spice-devel,v2,2/2] spiceqxl: Reject invalid boolean values, just like for other options.

Submitted by Francois Gouget on Aug. 31, 2015, 1:29 p.m.

Details

Message ID alpine.DEB.2.20.1508311527420.31956@amboise
State New
Headers show

Not browsing as part of any series.

Commit Message

Francois Gouget Aug. 31, 2015, 1:29 p.m.
Signed-off-by: Francois Gouget <fgouget@codeweavers.com>
---
 src/qxl_option_helpers.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/qxl_option_helpers.c b/src/qxl_option_helpers.c
index a42cc84..2aba677 100644
--- a/src/qxl_option_helpers.c
+++ b/src/qxl_option_helpers.c
@@ -49,6 +49,6 @@  int get_bool_option(OptionInfoPtr options, int option_index,
         return TRUE;
     }
 
-    fprintf(stderr, "spice: treating invalid boolean %s as true: %s\n", env_name, value);
-    return TRUE;
+    fprintf(stderr, "spice: invalid %s: %s\n", env_name, value);
+    exit(1);
 }

Comments

> 
> Signed-off-by: Francois Gouget <fgouget@codeweavers.com>
> ---
>  src/qxl_option_helpers.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/qxl_option_helpers.c b/src/qxl_option_helpers.c
> index a42cc84..2aba677 100644
> --- a/src/qxl_option_helpers.c
> +++ b/src/qxl_option_helpers.c
> @@ -49,6 +49,6 @@ int get_bool_option(OptionInfoPtr options, int
> option_index,
>          return TRUE;
>      }
>  
> -    fprintf(stderr, "spice: treating invalid boolean %s as true: %s\n",
> env_name, value);
> -    return TRUE;
> +    fprintf(stderr, "spice: invalid %s: %s\n", env_name, value);
> +    exit(1);
>  }
> --
> 2.5.0
> 

I think etiher we should merge first patch and discard the second or you should merge
the two patches together.

Frediano
On Mon, 31 Aug 2015, Frediano Ziglio wrote:
[...]
> I think etiher we should merge first patch and discard the second or you should merge
> the two patches together.

I think it would be ok to commit them separately as a way to make the 
policy change more obvious. But the idea is also that you now get to 
decide whether to comit the second patch right away, announce an 
upcoming policy change regarding the handling of booleans and comit it 6 
months or a year from, now, or drop it entirely.

And if a single patch is the preferred option, then that's available 
too:
http://lists.freedesktop.org/archives/spice-devel/2015-June/020172.html


I don't have a strong opinion on this so the ball is in your camp.
On Tue, Sep 01, 2015 at 12:44:51AM +0200, Francois Gouget wrote:
> On Mon, 31 Aug 2015, Frediano Ziglio wrote:
> [...]
> > I think etiher we should merge first patch and discard the second or you should merge
> > the two patches together.
> 
> I think it would be ok to commit them separately as a way to make the 
> policy change more obvious.


Yes, imo it's better this way.

> But the idea is also that you now get to 
> decide whether to comit the second patch right away, announce an 
> upcoming policy change regarding the handling of booleans and comit it 6 
> months or a year from, now, or drop it entirely.
> 
> And if a single patch is the preferred option, then that's available 
> too:
> http://lists.freedesktop.org/archives/spice-devel/2015-June/020172.html
> 
> 
> I don't have a strong opinion on this so the ball is in your camp.
> 

I've pushed this series now,

Christophe