Re: avicap32 patch

[Juan Lang]

Hi Maarten,
> +#ifdef HAVE_LINUX_VIDEODEV_H
> +#include <stdio.h>
> +#include <sys/ioctl.h>
> +#include <sys/stat.h>
> +#include <errno.h>
> +#include <asm/types.h>
> +#include <linux/videodev.h>
> +#include <fcntl.h>
> +#include <unistd.h>
> +#include "winnls.h"
> +#endif

You need to include "winnls.h" unconditionally, capGetDriverDescriptionW
is always compiled with calls to MultiByteToWideChar.
> - sprintf(device, "/dev/video%i", devnum);

Why not use snprintf(device, sizeof(device) ...) instead?
> -
>  strcpy(name, caps.card);

Ug.  Please don't use strcpy.  Even though caps.card and name happen to be
declared of the same size, and caps.card is very likely null-terminated,
it takes a bit of searching around to make sure that's the case.  It's
easier to verify it's correct if you use lstrncpyA or memcpy instead. 
Since you NULL-terminate in the next line, I'd suggest memcpy.
> -
>  sprintf(version, "%s v%u.%u.%u",

It's also not obvious how large version is.  I'd recommend passing in the
length of the buffer and using snprintf instead.  You might as well pass
in the length of name, too.
> - strncpy(lpszName, devname, cbName);

You won't be able to use strncpy anymore, see Alexandre's commit:
http://www.winehq.org/hypermail/wine-cvs/2005/04/0275.html

--Juan

__________________________________________________
Do You Yahoo!?
Tired of spam?  Yahoo! Mail has the best spam protection around 
http://mail.yahoo.com

avicap32 patch

[Maarten Lankhorst]

Juan Lang wrote:
> Hi Maarten,
>
> You need to include "winnls.h" unconditionally, capGetDriverDescriptionW
> is always compiled with calls to MultiByteToWideChar.

Oops, didn't see it, my fault -- fixed
> > - sprintf(device, "/dev/video%i", devnum);
>
> Why not use snprintf(device, sizeof(device) ...) instead?

it's not like it can overflow.. but ok fixed
> > -
> >  strcpy(name, caps.card);
>
> Ug.  Please don't use strcpy.  Even though caps.card and name happen to be
> declared of the same size, and caps.card is very likely null-terminated,
> it takes a bit of searching around to make sure that's the case.  It's
> easier to verify it's correct if you use lstrncpyA or memcpy instead. 
> Since you NULL-terminate in the next line, I'd suggest memcpy.
> > -
> >  sprintf(version, "%s v%u.%u.%u",
>
> It's also not obvious how large version is.  I'd recommend passing in the
> length of the buffer and using snprintf instead.  You might as well pass
> in the length of name, too.

Fixed those 2
> > - strncpy(lpszName, devname, cbName);
>
> You won't be able to use strncpy anymore, see Alexandre's commit:
> http://www.winehq.org/hypermail/wine-cvs/2005/04/0275.html
>
> --Juan

When I just compiled cvs again, I noticed the error message hehe
> Do You Yahoo!?
> Tired of spam?  Yahoo! Mail has the best spam protection around 
> http://mail.yahoo.com

Tired of annoying mailers that add such messages ;)

Also, thanks to a gentoo user I found out why #include 
<linux/videodev.h> sometimes fails,
and added a fixing #include <sys/time.h> to the configure scripts
I suppose the patch is ready for submitting in the cvs this time now :)