[QCAP] Final patch

Dimi Paun dimi at lattica.com
Thu May 19 20:20:25 CDT 2005


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...

-- 
Dimi Paun <dimi at lattica.com>
Lattica, Inc.




More information about the wine-devel mailing list