On Wednesday 14 May 2008 04:29:32 Andy Walls wrote:
> 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',
The snprintf implementation in the kernel (lib/vsprintf.c) will always
end the string with a '\0'. Strictly speaking it is not 100% C-library
compatible, but it is much safer that way.
Regards,
Hans
> 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-1
> > >3 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-1
> > >3 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
