[PATCH v3] parser: be more picky for integer values

Tiago Vignatti tiago.vignatti at linux.intel.com
Fri Apr 20 04:33:47 PDT 2012


On 04/20/2012 12:02 PM, Pekka Paalanen wrote:
> On Fri, 16 Mar 2012 15:12:14 -0300
> Tiago Vignatti<tiago.vignatti at intel.com>  wrote:
>
>> It was silently accepting "-i=3", "-i=3/2", "--idle-time=*3" and similar
>> unwanted type of arguments, not changing anything internally; this
>> gives the wrong impression for the user. Now it explicitly warns.
>>
>> Signed-off-by: Tiago Vignatti<tiago.vignatti at intel.com>
>> ---
>> Pekka, I guess now it's alright, please take a look. If it's good to go, you
>> would mind to stamp a Reviewed-By for the other patches in this series as
>> well?
>
> Hi,
>
> I sent my r-b, but looks like this patch was never committed. Any
> reason why?
>
> Also, what happened to the other patches in this series?

oh, thanks for reminding Pekka! I rebased them with master and put here:

     http://cgit.freedesktop.org/~vignatti/weston/log/?h=options

the patches are:
   parser: be more picky for integer values
   compositor: fix option types
   compositor: dump command line options


For the first there, Kristian was reluctant a bit and mentioned that 
it's a lot of code for not much usage. So those functions would be more 
useful and valuable if we could use them in config-parser.c as well. I'm 
okay with that; we can do it.

Kristian opposed the second patch. He claimed that what matters isn't 
what values an option may or may not take, but what the c type that 
stores the value is. So the type of the option is not a check that the 
values makes sense. I agree, but I think we should rework a bit then the 
weston_option_type emum, probably removing 
WESTON_OPTION_UNSIGNED_INTEGER or something.

Then, the third patch I guess no one objected it. It's simple and does 
what intended. But you didn't give your r-b either there, so :)


What do you think? Can you please comment now so I can make the right 
fixes accordingly?

Tiago


More information about the wayland-devel mailing list