[PATCH] xfree86: Add option parsing for percent options.
Dan Nicholson
dbn.lists at gmail.com
Thu May 13 15:19:12 PDT 2010
On Wed, May 12, 2010 at 11:07 PM, Peter Hutterer
<peter.hutterer at who-t.net> wrote:
> In some cases, an option of "50%" would be preferable over fixed value
> configuration - especially if the actual values are autoprobed.
> Add a new set of functions to parse percent values from configurations.
>
> The percent value parsing differs slightly - if the option is not to marked
> as used (e.g. xf86CheckPercentOption()), no warning is emitted to the log
> file if the value is not a percent value. This allows double-options (either
> as % or as absolute number) without warnings.
>
> Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> ---
> This would be quite useful in a few input drivers, especially now that we
> have xorg.conf snippets. Best example here are the synaptics Area options,
> where it'd be much easier to just specify the bottom area as 80% instead of
> guessing what the touchpad may have.
>
> One question is of course if we should allow the values to be floats instead
> of ints, not sure if we need something like 0.2% but I guess it might not
> hurt, either.
>
> hw/xfree86/common/xf86Opt.h | 4 +++
> hw/xfree86/common/xf86Option.c | 48 ++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 52 insertions(+), 0 deletions(-)
>
> diff --git a/hw/xfree86/common/xf86Opt.h b/hw/xfree86/common/xf86Opt.h
> index ce3d767..479c513 100644
> --- a/hw/xfree86/common/xf86Opt.h
> +++ b/hw/xfree86/common/xf86Opt.h
> @@ -51,6 +51,7 @@ typedef enum {
> OPTV_ANYSTR, /* Any string, including an empty one */
> OPTV_REAL,
> OPTV_BOOLEAN,
> + OPTV_PERCENT,
> OPTV_FREQ
> } OptionValueType;
>
> @@ -72,10 +73,12 @@ extern _X_EXPORT int xf86SetIntOption(pointer optlist, const char *name, int def
> extern _X_EXPORT double xf86SetRealOption(pointer optlist, const char *name, double deflt);
> extern _X_EXPORT char *xf86SetStrOption(pointer optlist, const char *name, char *deflt);
> extern _X_EXPORT int xf86SetBoolOption(pointer list, const char *name, int deflt );
> +extern _X_EXPORT int xf86SetPercentOption(pointer list, const char *name, int deflt );
> extern _X_EXPORT int xf86CheckIntOption(pointer optlist, const char *name, int deflt);
> extern _X_EXPORT double xf86CheckRealOption(pointer optlist, const char *name, double deflt);
> extern _X_EXPORT char *xf86CheckStrOption(pointer optlist, const char *name, char *deflt);
> extern _X_EXPORT int xf86CheckBoolOption(pointer list, const char *name, int deflt );
> +extern _X_EXPORT int xf86CheckPercentOption(pointer list, const char *name, int deflt );
> extern _X_EXPORT pointer xf86AddNewOption(pointer head, const char *name, const char *val );
> extern _X_EXPORT pointer xf86NewOption(char *name, char *value );
> extern _X_EXPORT pointer xf86NextOption(pointer list );
> @@ -109,5 +112,6 @@ extern _X_EXPORT char *xf86NormalizeName(const char *s);
> extern _X_EXPORT pointer xf86ReplaceIntOption(pointer optlist, const char *name, const int val);
> extern _X_EXPORT pointer xf86ReplaceRealOption(pointer optlist, const char *name, const double val);
> extern _X_EXPORT pointer xf86ReplaceBoolOption(pointer optlist, const char *name, const Bool val);
> +extern _X_EXPORT pointer xf86ReplacePercentOption(pointer optlist, const char *name, const int val);
> extern _X_EXPORT pointer xf86ReplaceStrOption(pointer optlist, const char *name, const char* val);
> #endif
> diff --git a/hw/xfree86/common/xf86Option.c b/hw/xfree86/common/xf86Option.c
> index a2868bf..ab86ace 100644
> --- a/hw/xfree86/common/xf86Option.c
> +++ b/hw/xfree86/common/xf86Option.c
> @@ -223,6 +223,18 @@ LookupBoolOption(pointer optlist, const char *name, int deflt, Bool markUsed)
> return deflt;
> }
>
> +static int
> +LookupPercentOption(pointer optlist, const char *name, int deflt, Bool markUsed)
> +{
> + OptionInfoRec o;
> +
> + o.name = name;
> + o.type = OPTV_PERCENT;
> + if (ParseOptionValue(-1, optlist, &o, markUsed))
> + deflt = o.value.num;
> + return deflt;
> +}
> +
> /* These xf86Set* functions are intended for use by non-screen specific code */
>
> int
> @@ -252,6 +264,12 @@ xf86SetBoolOption(pointer optlist, const char *name, int deflt)
> return LookupBoolOption(optlist, name, deflt, TRUE);
> }
>
> +int
> +xf86SetPercentOption(pointer optlist, const char *name, int deflt)
> +{
> + return LookupPercentOption(optlist, name, deflt, TRUE);
> +}
> +
> /*
> * These are like the Set*Option functions, but they don't mark the options
> * as used.
> @@ -283,6 +301,12 @@ xf86CheckBoolOption(pointer optlist, const char *name, int deflt)
> return LookupBoolOption(optlist, name, deflt, FALSE);
> }
>
> +
> +int
> +xf86CheckPercentOption(pointer optlist, const char *name, int deflt)
> +{
> + return LookupPercentOption(optlist, name, deflt, FALSE);
> +}
> /*
I think you wanted a newline before the comment instead of two after
the previous function. :)
> * addNewOption() has the required property of replacing the option value
> * if the option is already present.
> @@ -310,6 +334,14 @@ xf86ReplaceBoolOption(pointer optlist, const char *name, const Bool val)
> }
>
> pointer
> +xf86ReplacePercentOption(pointer optlist, const char *name, const int val)
> +{
> + char tmp[16];
> + sprintf(tmp, "%d%%", val);
> + return xf86AddNewOption(optlist,name,tmp);
> +}
Why is the buffer length 16? Seems you could limit to 5 by sanity
checking 0 <= val <= 100. Oh, I guess when the new option is added it
could be checked there.
> +
> +pointer
> xf86ReplaceStrOption(pointer optlist, const char *name, const char* val)
> {
> return xf86AddNewOption(optlist,name,val);
> @@ -533,6 +565,22 @@ ParseOptionValue(int scrnIndex, pointer options, OptionInfoPtr p,
> p->found = FALSE;
> }
> break;
> + case OPTV_PERCENT:
> + {
> + char tmp[2];
> + /* awkward match, but %% doesn't increase the match counter,
> + * hence 100 looks the same as 100% to the caller of sccanf
> + */
> + if (sscanf(s, "%ld%1[%]", &p->value.num, tmp) != 2) {
> + if (markUsed)
> + xf86DrvMsg(scrnIndex, X_WARNING,
> + "Option \"%s\" requires a percent value\n", p->name);
> + p->found = FALSE;
Why is the warning being suppressed with markUsed? Because we can't
trust the formatting of the value or so that CheckPercent doesn't see
a warning or something else?
> + } else {
> + p->found = TRUE;
Would be nice to sanity check 0 <= val <= 100 here so that users didn't have to.
Looks nice, though. Assuming the warning suppression is explained sufficiently,
Reviewed-by: Dan Nicholson <dbn.lists at gmail.com>
--
Dan
More information about the xorg-devel
mailing list