On Tue, 2008-05-13 at 20:29 +0200, Hans Verkuil wrote:
> On Tuesday 13 May 2008 19:55:05 Jean Delvare wrote:
> > snprinf() takes the trailing \0 into account in its length
> > calculations, so there is no need to subtract 1 to the buffer size.
> >
> > Signed-off-by: Jean Delvare <[EMAIL PROTECTED]>
> 
> Thanks, merged in my tree.

Please consider that snprintf() will *not* add the terminating '\0', if
it must truncate the expanded source string because there is not enough
room in the destination (In our specific case, if cx->num < -9 or
cx->num > 99 somehow).  

Either way you make the call to snprintf(), the snprintf() statement
should be followed with this:

        cx->name [sizeof(cx->name)-1] = '\0';

The original snprintf() call used an unreliable method of insuring that
at least one trailing '\0' remained at the end of the destination
string: it *assumed* the destination string buffer was already all 0's
(or '\0's), and then never let snprintf() overwrite the final '\0' that
was *assumed* to be there.

The new version of the call to snprintf() will allow snprintf() to
overwrite any final '\0' that may preexist in the destination string
buffer.  

This makes an explicit write of '\0' in the last character absolutely
necessary, if you cannot guarantee that cx->num will never inadvertently
take on a value outside of [-9, 99].  If you can guarantee that, then
there is no reason to use snprintf() over sprintf() in the first place.


Reference:
$ man snprintf

Regards,
Andy

> 
> Similar snprintf constructs are in e.g. saa7115.c and saa7127.c. These 
> all concern client->name. Did you clean that up in your i2c patches? Or 
> shall I fix them?
> 
> Regards,
> 
>       Hans
> 
> > ---
> >  drivers/media/video/cx18/cx18-driver.c |    2 +-
> >  drivers/media/video/ivtv/ivtv-driver.c |    2 +-
> >  2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > ---
> > linux-2.6.26-rc2.orig/drivers/media/video/cx18/cx18-driver.c        2008-05-
> >12 08:22:04.000000000 +0200 +++
> > linux-2.6.26-rc2/drivers/media/video/cx18/cx18-driver.c     2008-05-13
> > 19:12:56.000000000 +0200 @@ -620,7 +620,7 @@ static int __devinit
> > cx18_probe(struct p cx18_cards[cx18_cards_active] = cx;
> >     cx->dev = dev;
> >     cx->num = cx18_cards_active++;
> > -   snprintf(cx->name, sizeof(cx->name) - 1, "cx18-%d", cx->num);
> > +   snprintf(cx->name, sizeof(cx->name), "cx18-%d", cx->num);
> >     CX18_INFO("Initializing card #%d\n", cx->num);
> >
> >     spin_unlock(&cx18_cards_lock);
> > ---
> > linux-2.6.26-rc2.orig/drivers/media/video/ivtv/ivtv-driver.c        2008-05-
> >04 09:49:51.000000000 +0200 +++
> > linux-2.6.26-rc2/drivers/media/video/ivtv/ivtv-driver.c     2008-05-13
> > 19:12:51.000000000 +0200 @@ -1015,7 +1015,7 @@ static int __devinit
> > ivtv_probe(struct p ivtv_cards[ivtv_cards_active] = itv;
> >     itv->dev = dev;
> >     itv->num = ivtv_cards_active++;
> > -   snprintf(itv->name, sizeof(itv->name) - 1, "ivtv%d", itv->num);
> > +   snprintf(itv->name, sizeof(itv->name), "ivtv%d", itv->num);
> >     IVTV_INFO("Initializing card #%d\n", itv->num);
> >
> >     spin_unlock(&ivtv_cards_lock);
> 
> 
> 
> _______________________________________________
> ivtv-devel mailing list
> [email protected]
> http://ivtvdriver.org/mailman/listinfo/ivtv-devel
> 


_______________________________________________
ivtv-devel mailing list
[email protected]
http://ivtvdriver.org/mailman/listinfo/ivtv-devel

Reply via email to