Re: [Aiptektablet-users] [PATCH 2.6.20 1/1] usb/input updatedaiptek tablet driver

On Tuesday 03 April 2007 17:25, René van Paassen wrote:
> -static ssize_t show_tabletProductId(struct device *dev, struct device_attribute *attr, char *buf)
> -{
> -	struct aiptek *aiptek = dev_get_drvdata(dev);
> -
> -	if (aiptek == NULL)
> +	if (aiptek == NULL) {
>  		return 0;
> +	}

Does it make any sense to check for aiptek != NULL? sysfs attributes are now
orphaned upon removal request so unless a device is bound to aiptek driver
there is no way its show/store methods going to be called.

Plus we don't normally use open/close braces for single statements.

>  
>  /***********************************************************************
>   * support routines for the 'pointer_mode' file. Note that this file
> @@ -1103,22 +1197,22 @@
>  
>  	switch (aiptek->curSetting.pointerMode) {
>  	case AIPTEK_POINTER_ONLY_STYLUS_MODE:
> -		s = "stylus";
> +		s = pStylus;
>  		break;
>  
>  	case AIPTEK_POINTER_ONLY_MOUSE_MODE:
> -		s = "mouse";
> +		s = pMouse;
>  		break;
>  
>  	case AIPTEK_POINTER_EITHER_MODE:
> -		s = "either";
> +		s = pEither;
>  		break;
>  
>  	default:
> -		s = "unknown";
> +		s = pUnknown;
>  		break;
>  	}
> -	return snprintf(buf, PAGE_SIZE, "%s\n", s);
> +	return snprintf(buf, PAGE_SIZE, pString, s);
>  }
>  
>  static ssize_t
> @@ -1128,12 +1222,12 @@
>  	if (aiptek == NULL)
>  		return 0;
>  
> -	if (strcmp(buf, "stylus") == 0) {
> +	if (strncmp(buf, pStylus, ARRAY_SIZE(pStylus)) == 0) {
>  		aiptek->newSetting.pointerMode =
>  		    AIPTEK_POINTER_ONLY_STYLUS_MODE;
> -	} else if (strcmp(buf, "mouse") == 0) {
> +	} else if (strncmp(buf, pMouse, ARRAY_SIZE(pMouse)) == 0) {
>  		aiptek->newSetting.pointerMode = AIPTEK_POINTER_ONLY_MOUSE_MODE;
> -	} else if (strcmp(buf, "either") == 0) {
> +	} else if (strncmp(buf, pEither, ARRAY_SIZE(pEither)) == 0) {
>  		aiptek->newSetting.pointerMode = AIPTEK_POINTER_EITHER_MODE;
>  	}


I think if you had an array of string/constant pairs this code and above
would be much shorter and maintainable.

>  	return count;
> @@ -1157,18 +1251,18 @@
>  
>  	switch (aiptek->curSetting.coordinateMode) {
>  	case AIPTEK_COORDINATE_ABSOLUTE_MODE:
> -		s = "absolute";
> +	        s = pAbsolute;
>  		break;
>  
>  	case AIPTEK_COORDINATE_RELATIVE_MODE:
> -		s = "relative";
> +		s = pRelative;
>  		break;
>  
>  	default:
> -		s = "unknown";
> +		s = pUnknown;
>  		break;
>  	}
> -	return snprintf(buf, PAGE_SIZE, "%s\n", s);
> +	return snprintf(buf, PAGE_SIZE, pString, s);
>  }
>  
>  static ssize_t
> @@ -1178,11 +1272,11 @@
>  	if (aiptek == NULL)
>  		return 0;
>  
> -	if (strcmp(buf, "absolute") == 0) {
> -		aiptek->newSetting.pointerMode =
> +	if (strncmp(buf, pAbsolute, ARRAY_SIZE(pAbsolute)) == 0) {
> +		aiptek->newSetting.coordinateMode =
>  		    AIPTEK_COORDINATE_ABSOLUTE_MODE;
> -	} else if (strcmp(buf, "relative") == 0) {
> -		aiptek->newSetting.pointerMode =
> +	} else if (strncmp(buf, pRelative, ARRAY_SIZE(pRelative)) == 0) {
> +		aiptek->newSetting.coordinateMode =
>  		    AIPTEK_COORDINATE_RELATIVE_MODE;
>  	}

Same here.

>  	return count;
> @@ -1206,38 +1300,38 @@
>  
>  	switch (TOOL_BUTTON(aiptek->curSetting.toolMode)) {
>  	case AIPTEK_TOOL_BUTTON_MOUSE_MODE:
> -		s = "mouse";
> +		s = pMouse;
>  		break;
>  
>  	case AIPTEK_TOOL_BUTTON_ERASER_MODE:
> -		s = "eraser";
> +		s = pEraser;
>  		break;
>  
>  	case AIPTEK_TOOL_BUTTON_PENCIL_MODE:
> -		s = "pencil";
> +		s = pPencil;
>  		break;
>  
>  	case AIPTEK_TOOL_BUTTON_PEN_MODE:
> -		s = "pen";
> +		s = pPen;
>  		break;
>  
>  	case AIPTEK_TOOL_BUTTON_BRUSH_MODE:
> -		s = "brush";
> +		s = pBrush;
>  		break;
>  
>  	case AIPTEK_TOOL_BUTTON_AIRBRUSH_MODE:
> -		s = "airbrush";
> +		s = pAirbrush;
>  		break;
>  
>  	case AIPTEK_TOOL_BUTTON_LENS_MODE:
> -		s = "lens";
> +		s = pLens;
>  		break;
>  
>  	default:
> -		s = "unknown";
> +		s = pUnknown;
>  		break;
>  	}
> -	return snprintf(buf, PAGE_SIZE, "%s\n", s);
> +	return snprintf(buf, PAGE_SIZE, pString, s);
>  }
>  
>  static ssize_t
> @@ -1247,19 +1341,19 @@
>  	if (aiptek == NULL)
>  		return 0;
>  
> -	if (strcmp(buf, "mouse") == 0) {
> +	if (strncmp(buf, pMouse, ARRAY_SIZE(pMouse)) == 0) {
>  		aiptek->newSetting.toolMode = AIPTEK_TOOL_BUTTON_MOUSE_MODE;
> -	} else if (strcmp(buf, "eraser") == 0) {
> +	} else if (strncmp(buf, pEraser, ARRAY_SIZE(pEraser)) == 0) {
>  		aiptek->newSetting.toolMode = AIPTEK_TOOL_BUTTON_ERASER_MODE;
> -	} else if (strcmp(buf, "pencil") == 0) {
> +	} else if (strncmp(buf, pPencil, ARRAY_SIZE(pPencil)) == 0) {
>  		aiptek->newSetting.toolMode = AIPTEK_TOOL_BUTTON_PENCIL_MODE;
> -	} else if (strcmp(buf, "pen") == 0) {
> +	} else if (strncmp(buf, pPen, ARRAY_SIZE(pPen)) == 0) {
>  		aiptek->newSetting.toolMode = AIPTEK_TOOL_BUTTON_PEN_MODE;
> -	} else if (strcmp(buf, "brush") == 0) {
> +	} else if (strncmp(buf, pBrush, ARRAY_SIZE(pBrush)) == 0) {
>  		aiptek->newSetting.toolMode = AIPTEK_TOOL_BUTTON_BRUSH_MODE;
> -	} else if (strcmp(buf, "airbrush") == 0) {
> +	} else if (strncmp(buf, pAirbrush, ARRAY_SIZE(pAirbrush)) == 0) {
>  		aiptek->newSetting.toolMode = AIPTEK_TOOL_BUTTON_AIRBRUSH_MODE;
> -	} else if (strcmp(buf, "lens") == 0) {
> +	} else if (strncmp(buf, pLens, ARRAY_SIZE(pLens)) == 0) {
>  		aiptek->newSetting.toolMode = AIPTEK_TOOL_BUTTON_LENS_MODE;
>  	}
>  

And here.

-- 
Dmitry

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
[email protected]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel