Re: [Qemu-devel] [5531] Replace uses of strncpy (a GNU extension) with Qemu pstrcpy

["Blue Swirl" <blauwirbel@gmail.com>]

On 10/25/08, malc <av1474@comtv.ru> wrote:
> On Sat, 25 Oct 2008, Blue Swirl wrote:
>
>
> > Revision: 5531
> >
> http://svn.sv.gnu.org/viewvc/?view=rev&root=qemu&revision=5531
> > Author:   blueswir1
> > Date:     2008-10-25 11:21:28 +0000 (Sat, 25 Oct 2008)
> >
> >
>
>  [..snip..]
>
>
> > Modified: trunk/block-vvfat.c
> >
> ===================================================================
> > --- trunk/block-vvfat.c 2008-10-25 11:19:14 UTC (rev 5530)
> > +++ trunk/block-vvfat.c 2008-10-25 11:21:28 UTC (rev 5531)
> > @@ -625,7 +625,7 @@
> >
> >    entry=array_get_next(&(s->directory));
> >    memset(entry->name,0x20,11);
> > -    strncpy((char*)entry->name,filename,i);
> > +    pstrcpy((char*)entry->name, i, filename);
> >
> >    if(j > 0)
> >        for (i = 0; i < 3 && filename[j+1+i]; i++)
> >
>
>  This is wrong and broke vvfat:
>
>  a. pstrcpy is abused, second argument should be the size of the output
>    buffer, not the length of the second argument

It's actually not the raw length, but MAX(length, 8).

>  b. even if replaced with pstrcpy(.., meta_sizeof(entry->name), ...)
>    it would have been still wrong, since we must _i_ elements of
>    entry->name and NOT try to zero terminate the output buffer
>
>  I think this was one of those rare cases when strncpy was used in
>  accordance with it's definition and not what programmer was imagining.
>
>  I think plain memcpy(...,filename,i) is the most warranted thing here.

Yes, that should do the right thing.

I think the cutils.c needs some more commenting to document these corner cases.