Re: [QCAP] Final patch

[Dimi Paun]

On Fri, 2005-05-20 at 00:42 +0200, Maarten Lankhorst wrote:
> m3h, v4l driver for vfwcapture.. please leave the ERR notice on 
> initialisation intact, thank you :)

I think you've overusing ERR(). ERR() should be called
to signal an internal error such as inconsistent state,
not something that the function is designed to handle.
> -
>        capBox->grab_buf = CoTaskMemAlloc(sizeof(struct video_mmap) * capBox->buffers);
> -
>        if(!capBox->grab_buf) {
> -
>            ERR("Out of memory?\n");
> -
>            munmap(capBox->pmap, capBox->gb_buffers.size);
> -
>            return E_OUTOFMEMORY;
> -
>        }

The function knows how to deal with this condition gracefully,
just get rid of the ERR().
> -
>     if (!capBox->grab_data)
> -
>     {
> -
>        ERR("%p: Out of memory?\n", capBox);
> -
>        return E_OUTOFMEMORY;
> -
>     }

Ditto. Also the code is more clear if you just do:
            if (!capBox->grab_data) return E_OUTOFMEMORY;
> - if (!capBox) {
> -
>  ERR("Out of memory\n");
> -
>  return E_OUTOFMEMORY;
> - }

Ditto.
> - if (stat (device, &st) == -1) {
> -
>  ERR("%s: %s\n", device, strerror(errno));
> -
>  CoTaskMemFree(capBox);
> -
>  return E_FAIL;
> - }

Why is this a ERR()?
> -
> - if (!S_ISCHR (st.st_mode)) {
> -
>  ERR("%s: Not a device\n", device);
> -
>  CoTaskMemFree(capBox);
> -
>  return E_FAIL;
> - }

And why this?
> -
> - capBox->fd = open(device, O_RDWR | O_NONBLOCK);
> - if (capBox->fd == -1) {
> -
>  ERR("%s: Failed to open: %s\n", device, strerror(errno));
> -
>  CoTaskMemFree(capBox);
> -
>  return E_FAIL;
> - }

This is a TRACE or WARN at most.
> -
>  ERR("Tinkerer detected? Thou shalt not define HAVE_V4L2\n");
> -
>  CoTaskMemFree(capBox);
> -
>  close(capBox->fd);
> -
>  return E_FAIL;

This is a FIXME.

And so on...