Re: camcontrol WARNS6: take 5

[Chris Pressey <cpressey@xxxxxxxxxxxxxxx>]

On Mon, 10 Jan 2005 22:13:39 -0500
Larry Lansing <lansil@xxxxxxxxxxxxx> wrote:

> The patch was getting too long to be reasonable.  This one
> is just WARNS6 and the function declarations cleanup.  I Can
> trim it down to just WARNS6 in one patch, if needed.
> 
> --
> Larry Lansing
> 

Overall, it looks good.  Two things:

1.  I still got a 'signed versus unsigned' warning from the
    RESOLUTION_MAX macro.  It was only being used twice (with
    the same argument,) so I expanded it and used a real
    variable for it instead, and that made the warning go away.
    Patch (over and on top of your patch) attached.

2.  It seems GCC accepts
       int foo(__unused int arg)
    just as easily as
       int foo(int arg __unused)
    I've been using the first version, this patch uses the
    second version, and style(9) is silent on the issue.
    Does the DragonFly community have a preference?

-Chris

--- modeedit.c	2005-01-10 20:55:09.000000000 -0800
+++ modeedit.c.mine	2005-01-10 20:56:54.000000000 -0800
@@ -234,15 +234,7 @@
 	char *convertend;	/* End-of-conversion pointer. */
 	int ival;		/* New integral value. */
 	int resolution;		/* Resolution in bits for integer conversion. */
-
-/*
- * Macro to determine the maximum value of the given size for the current
- * resolution.
- * XXX Lovely x86's optimize out the case of shifting by 32 and gcc doesn't
- *     currently workaround it (even for int64's), so we have to kludge it.
- */
-#define	RESOLUTION_MAX(size) ((resolution * (size) == 32)? 		\
-	0xffffffff: (1 << (resolution * (size))) - 1)
+	int resolution_max;	/* Maximum resolution for modepage's size. */
 
 	assert(newvalue != NULL);
 	if (*newvalue == '\0')
@@ -262,8 +254,21 @@
 		ival = (int)strtol(newvalue, &convertend, 0);
 		if (*convertend != '\0')
 			returnerr(EINVAL);
-		if (ival > RESOLUTION_MAX(dest->size) || ival < 0) {
-			int newival = (ival < 0)? 0: RESOLUTION_MAX(dest->size);
+
+		/*
+		 * Determine the maximum value of the given size for the
+		 * current resolution.
+		 * XXX Lovely x86's optimize out the case of shifting by 32,
+		 * and gcc doesn't currently workaround it (even for int64's),
+		 * so we have to kludge it.
+		 */
+		if (resolution * dest->size == 32)
+			resolution_max = 0xffffffff;
+		else
+			resolution_max = (1 << (resolution * dest->size)) - 1;
+
+		if (ival > resolution_max || ival < 0) {
+			int newival = (ival < 0) ? 0 : resolution_max;
 			warnx("value %d is out of range for entry %s; clipping "
 			    "to %d", ival, name, newival);
 			ival = newival;
@@ -310,7 +315,6 @@
 	}
 
 	return (0);
-#undef RESOLUTION_MAX
 }
 
 static void