[PATCH:xpr] Use strncmp() instead of bcmp() to compare string subsets

Matthieu Herrb matthieu at herrb.eu
Fri Aug 8 07:30:43 PDT 2014


On 08/08/2014 04:51 AM, Alan Coopersmith wrote:
> Signed-off-by: Alan Coopersmith <alan.coopersmith at oracle.com>

Hi,

Here it seems to me that strcmp() would be safe. While it changes the 
behaviour a bit, it seems more logical to only accept exact matches on 
the options (and not anything with random suffixes after the initial 
option name).

Moreover I got bored by the useless micro-optimisations of the option 
parser and removed them.

What do you think of the attached patch instead?

> ---
>   xpr.c |   74 ++++++++++++++++++++++++++++++++---------------------------------
>   1 file changed, 37 insertions(+), 37 deletions(-)
>
> diff --git a/xpr.c b/xpr.c
> index 07a8a30..30c7873 100644
> --- a/xpr.c
> +++ b/xpr.c
> @@ -573,7 +573,7 @@ void parse_args(
>   	len = strlen(*argv);
>   	switch (argv[0][1]) {
>   	case 'a':		/* -append <filename> */
> -	    if (!bcmp(*argv, "-append", len)) {
> +	    if (!strncmp(*argv, "-append", len)) {
>   		argc--; argv++;
>   		if (argc == 0) missing_arg(arg);
>   		output_filename = *argv;
> @@ -585,9 +585,9 @@ void parse_args(
>   	case 'c':		/* -compact | -cutoff <intensity> */
>   	    if (len <= 2 )
>   		unknown_arg(arg);
> -	    if (!bcmp(*argv, "-compact", len)) {
> +	    if (!strncmp(*argv, "-compact", len)) {
>   		*flags |= F_COMPACT;
> -	    } else if (!bcmp(*argv, "-cutoff", len)) {
> +	    } else if (!strncmp(*argv, "-cutoff", len)) {
>   		argc--; argv++;
>   		if (argc == 0) missing_arg(arg);
>   		*cutoff = min((atof(*argv) / 100.0 * 0xFFFF), 0xFFFF);
> @@ -598,35 +598,35 @@ void parse_args(
>   	case 'd':	/* -density <num> | -device <dev> | -dump */
>   	    if (len <= 2)
>   		unknown_arg(arg);
> -	    if (!bcmp(*argv, "-dump", len)) {
> +	    if (!strncmp(*argv, "-dump", len)) {
>   		*flags |= F_DUMP;
>   	    } else if (len <= 3) {
>   		unknown_arg(arg);
> -	    } else if (!bcmp(*argv, "-density", len)) {
> +	    } else if (!strncmp(*argv, "-density", len)) {
>   		argc--; argv++;
>   		if (argc == 0) missing_arg(arg);
>   		*density = atoi(*argv);
> -	    } else if (!bcmp(*argv, "-device", len)) {
> +	    } else if (!strncmp(*argv, "-device", len)) {
>   		argc--; argv++;
>   		if (argc == 0) missing_arg(arg);
>   		len = strlen(*argv);
>   		if (len < 2)
>   		    unknown_arg(arg);
> -		if (!bcmp(*argv, "ln03", len)) {
> +		if (!strncmp(*argv, "ln03", len)) {
>   		    *device = LN03;
> -		} else if (!bcmp(*argv, "la100", len)) {
> +		} else if (!strncmp(*argv, "la100", len)) {
>   		    *device = LA100;
> -		} else if (!bcmp(*argv, "ps", len)) {
> +		} else if (!strncmp(*argv, "ps", len)) {
>   		    *device = PS;
> -		} else if (!bcmp(*argv, "lw", len)) {
> +		} else if (!strncmp(*argv, "lw", len)) {
>   		    *device = PS;
> -		} else if (!bcmp(*argv, "pp", len)) {
> +		} else if (!strncmp(*argv, "pp", len)) {
>   		    *device = PP;
> -		} else if (!bcmp(*argv, "ljet", len)) {
> +		} else if (!strncmp(*argv, "ljet", len)) {
>   		    *device = LJET;
> -		} else if (!bcmp(*argv, "pjet", len)) {
> +		} else if (!strncmp(*argv, "pjet", len)) {
>   		    *device = PJET;
> -		} else if (!bcmp(*argv, "pjetxl", len)) {
> +		} else if (!strncmp(*argv, "pjetxl", len)) {
>   		    *device = PJETXL;
>   		} else
>   		    invalid_arg_value(arg, argv[0]);
> @@ -637,12 +637,12 @@ void parse_args(
>   	case 'g':		/* -gamma <float> | -gray <num> */
>   	    if (len <= 2)
>   		unknown_arg(arg);
> -	    if (!bcmp(*argv, "-gamma", len)) {
> +	    if (!strncmp(*argv, "-gamma", len)) {
>   		argc--; argv++;
>   		if (argc == 0) missing_arg(arg);
>   		*gamma = atof(*argv);
> -	    } else if (!bcmp(*argv, "-gray", len) ||
> -		!bcmp(*argv, "-grey", len)) {
> +	    } else if (!strncmp(*argv, "-gray", len) ||
> +		!strncmp(*argv, "-grey", len)) {
>   		argc--; argv++;
>   		if (argc == 0) missing_arg(arg);
>   		switch (atoi(*argv)) {
> @@ -666,11 +666,11 @@ void parse_args(
>   	case 'h':		/* -height <inches> | -header <string> */
>   	    if (len <= 3)
>   		unknown_arg(arg);
> -	    if (!bcmp(*argv, "-height", len)) {
> +	    if (!strncmp(*argv, "-height", len)) {
>   		argc--; argv++;
>   		if (argc == 0) missing_arg(arg);
>   		*height = (int)(300.0 * atof(*argv));
> -	    } else if (!bcmp(*argv, "-header", len)) {
> +	    } else if (!strncmp(*argv, "-header", len)) {
>   		argc--; argv++;
>   		if (argc == 0) missing_arg(arg);
>   		*header = *argv;
> @@ -681,9 +681,9 @@ void parse_args(
>   	case 'l':		/* -landscape | -left <inches> */
>   	    if (len <= 2)
>   		unknown_arg(arg);
> -	    if (!bcmp(*argv, "-landscape", len)) {
> +	    if (!strncmp(*argv, "-landscape", len)) {
>   		*flags |= F_LANDSCAPE;
> -	    } else if (!bcmp(*argv, "-left", len)) {
> +	    } else if (!strncmp(*argv, "-left", len)) {
>   		argc--; argv++;
>   		if (argc == 0) missing_arg(arg);
>   		*left = (int)(300.0 * atof(*argv));
> @@ -694,18 +694,18 @@ void parse_args(
>   	case 'n':		/* -nosixopt | -noff | -noposition */
>   	    if (len <= 3)
>   		unknown_arg(arg);
> -	    if (!bcmp(*argv, "-nosixopt", len)) {
> +	    if (!strncmp(*argv, "-nosixopt", len)) {
>   		*flags |= F_NOSIXOPT;
> -	    } else if (!bcmp(*argv, "-noff", len)) {
> +	    } else if (!strncmp(*argv, "-noff", len)) {
>   		*flags |= F_NOFF;
> -	    } else if (!bcmp(*argv, "-noposition", len)) {
> +	    } else if (!strncmp(*argv, "-noposition", len)) {
>   		*flags |= F_NPOSITION;
>   	    } else
>   		unknown_arg(arg);
>   	    break;
>
>   	case 'o':		/* -output <filename> */
> -	    if (!bcmp(*argv, "-output", len)) {
> +	    if (!strncmp(*argv, "-output", len)) {
>   		argc--; argv++;
>   		if (argc == 0) missing_arg(arg);
>   		output_filename = *argv;
> @@ -716,13 +716,13 @@ void parse_args(
>   	case 'p':		/* -portrait | -plane <n> */
>   	    if (len <= 2)
>   		unknown_arg(arg);
> -	    if (!bcmp(*argv, "-portrait", len)) {
> +	    if (!strncmp(*argv, "-portrait", len)) {
>   		*flags |= F_PORTRAIT;
> -	    } else if (!bcmp(*argv, "-plane", len)) {
> +	    } else if (!strncmp(*argv, "-plane", len)) {
>   		argc--; argv++;
>   		if (argc == 0) missing_arg(arg);
>   		*plane = atoi(*argv);
> -	    } else if (!bcmp(*argv, "-psfig", len)) {
> +	    } else if (!strncmp(*argv, "-psfig", len)) {
>   		*flags |= F_NPOSITION;
>   	    } else
>   		unknown_arg(arg);
> @@ -731,15 +731,15 @@ void parse_args(
>   	case 'r':		/* -render <type> | -report | -rv */
>   	    if (len <= 2)
>   		unknown_arg(arg);
> -	    if (!bcmp(*argv, "-rv", len)) {
> +	    if (!strncmp(*argv, "-rv", len)) {
>   		*flags |= F_INVERT;
>   	    } else if (len <= 3) {
>   		unknown_arg(arg);
> -	    } else if (!bcmp(*argv, "-render", len)) {
> +	    } else if (!strncmp(*argv, "-render", len)) {
>   		argc--; argv++;
>   		if (argc == 0) missing_arg(arg);
>   		*render = atoi(*argv);
> -	    } else if (!bcmp(*argv, "-report", len)) {
> +	    } else if (!strncmp(*argv, "-report", len)) {
>   		*flags |= F_REPORT;
>   	    } else
>   		unknown_arg(arg);
> @@ -748,13 +748,13 @@ void parse_args(
>   	case 's':	/* -scale <scale> | -slide | -split <n-pages> */
>   	    if (len <= 2)
>   		unknown_arg(arg);
> -	    if (!bcmp(*argv, "-scale", len)) {
> +	    if (!strncmp(*argv, "-scale", len)) {
>   		argc--; argv++;
>   		if (argc == 0) missing_arg(arg);
>   		*scale = atoi(*argv);
> -	    } else if (!bcmp(*argv, "-slide", len)) {
> +	    } else if (!strncmp(*argv, "-slide", len)) {
>   		*flags |= F_SLIDE;
> -	    } else if (!bcmp(*argv, "-split", len)) {
> +	    } else if (!strncmp(*argv, "-split", len)) {
>   		argc--; argv++;
>   		if (argc == 0) missing_arg(arg);
>   		*split = atoi(*argv);
> @@ -765,11 +765,11 @@ void parse_args(
>   	case 't':		/* -top <inches> | -trailer <string> */
>   	    if (len <= 2)
>   		unknown_arg(arg);
> -	    if (!bcmp(*argv, "-top", len)) {
> +	    if (!strncmp(*argv, "-top", len)) {
>   		argc--; argv++;
>   		if (argc == 0) missing_arg(arg);
>   		*top = (int)(300.0 * atof(*argv));
> -	    } else if (!bcmp(*argv, "-trailer", len)) {
> +	    } else if (!strncmp(*argv, "-trailer", len)) {
>   		argc--; argv++;
>   		if (argc == 0) missing_arg(arg);
>   		*trailer = *argv;
> @@ -786,7 +786,7 @@ void parse_args(
>   	    break;
>
>   	case 'w':		/* -width <inches> */
> -	    if (!bcmp(*argv, "-width", len)) {
> +	    if (!strncmp(*argv, "-width", len)) {
>   		argc--; argv++;
>   		if (argc == 0) missing_arg(arg);
>   		*width = (int)(300.0 * atof(*argv));
>

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Use-strcmp-to-compare-strings-and-simplify-options-p.patch
Type: text/x-patch
Size: 10645 bytes
Desc: not available
URL: <http://lists.x.org/archives/xorg-devel/attachments/20140808/754544d9/attachment-0001.bin>


More information about the xorg-devel mailing list