Re: lcd: fix memory leak, code cleanup

[Domen Puncer <domen@coderock.org>]

On 20/12/04 19:50 -0600, James Nelson wrote:
> This patch addresses the following issues:
> 
> Fix log-spamming and cryptic error messages, and add KERN_ constants.
> Convert some ints to unsigned ints.
> Add checks for CAP_SYS_ADMIN for FLASH_Burn and FLASH_Erase ioctls.
> Identify use of global variable.
> Fix memory leak in FLASH_Burn ioctl.
> Fix error return codes in lcd_ioctl().
> Move variable "index" in lcd_ioctl() to smaller scope to reduce memory usage.
> Convert cli()/sti() to spin_lock_irqsave()/spin_unlock_irqrestore().
> Fix legibility issues in FLASH_Burn ioctl.
>


> -				cli();
> +				spin_lock_irqsave(&lcd_lock, flags);
>  				for (index = 0; index < (128); index++) {
>  
>  					WRITE_FLASH(kFlash_Addr1,
> @@ -480,32 +485,30 @@
>  						    kFlash_Data2);
>  					WRITE_FLASH(kFlash_Addr1,
>  						    kFlash_Prog);
> -					*((volatile unsigned char *)
> -					  burn_addr) =
> -		 (volatile unsigned char) rom[index];
> -
> -					while ((!dqpoll
> -						(burn_addr,
> -						 (volatile unsigned char)
> -						 rom[index]))
> -					       && (!timeout(burn_addr))) {
> -					}
> +					*((volatile unsigned char *)burn_addr) =
> +					  (volatile unsigned char) rom[index];
> +
> +					while ((!dqpoll (burn_addr,
> +						(volatile unsigned char)
> +						rom[index])) &&
> +						(!timeout(burn_addr))) { }
>  					burn_addr++;
>  				}
> -				restore_flags(flags);
> -				if (*
> -				    ((volatile unsigned char *) (burn_addr
> -								 - 1)) ==
> -				    (volatile unsigned char) rom[index -
> -								 1]) {
> +				spin_unlock_irqrestore(&lcd_lock, flags);


Although this will work, i think local_irq_{disable,enable} is the right
solution (we don't protect any data, just make sure timings are right).

For making it SMP safe, easiest/sane solution seems semaphore in
lcd_dev, which is down_interruptible(), at beginning of read, write
and ioctl.

Comments?


	Domen