Subject: Re: CVS commit: src/sys
To: None <itojun@iijlab.net>
From: Klaus Klein <kleink@mibh.de>
List: tech-kern
Date: 04/24/2004 20:17:33
On Saturday 24 April 2004 19:36, itojun@iijlab.net wrote:
> >	the original code (with sprintf) is already broken, as sprintf()
> >	returns -1 on failure.  we just need to fix all of these
> >	cp += sprintf (or snprintf).
> 
> 	happy now?
> 
> itojun
> 
> 
> Index: ic/aic79xx.c
> ===================================================================
> RCS file: /cvsroot/src/sys/dev/ic/aic79xx.c,v
> retrieving revision 1.28
> diff -u -r1.28 aic79xx.c
> --- ic/aic79xx.c	21 Apr 2004 18:03:13 -0000	1.28
> +++ ic/aic79xx.c	24 Apr 2004 17:34:37 -0000
> @@ -8596,9 +8596,17 @@
>  		*cur_column = 0;
>  	}
>  	printed = snprintf(line, sizeof(line), "%s[0x%x]", name, value);
> +#ifdef DIAGNOSTIC
> +	if (printed > sizeof(line))
> +		panic("buffer too small in " __function__);
> +#endif

While I believe Simon's suggestion of panic() in the DIAGNOSTIC
case is a good idea (in the spirit of DIAGNOSTIC), why not take
this one step further and truncate in case of insufficient space
if !DIAGNOSTIC?  It's not as if these functions had a significant
performance impact (other than their ever-growing code size) so
I'd rather pay the expense of such a check than having memory
corrupted.

You've implemented most of the code already, and it would be a
shame to leave the code path promoting better coding practice
wrapped up inside #ifdef DIAGNOSTIC.


Thanks,
- Klaus