Hi Sakari,

On 07/26/2015 12:42 AM, Sakari Ailus wrote:
> Hi Hans,
> 
> On Fri, Jul 24, 2015 at 12:21:32PM +0200, Hans Verkuil wrote:
>> From: Hans Verkuil <hans.verk...@cisco.com>
>>
>> Add new helper functions that report back if this was the first open
>> or last close.
>>
>> Signed-off-by: Hans Verkuil <hans.verk...@cisco.com>
>> ---
> 
> ...
> 
>> @@ -65,11 +65,23 @@ void v4l2_fh_init(struct v4l2_fh *fh, struct 
>> video_device *vdev);
>>   */
>>  bool v4l2_fh_add(struct v4l2_fh *fh);
>>  /*
>> + * It allocates a v4l2_fh and inits and adds it to the video_device 
>> associated
>> + * with the file pointer. In addition it returns true if this was the first
>> + * open and false otherwise. The error code is returned in *err.
>> + */
>> +bool v4l2_fh_open_is_first(struct file *filp, int *err);
> 
> The new interface functions look a tad clumsy to me.
> 
> What would you think of returning the singularity value from v4l2_fh_open()
> straight away? Negative integers are errors, so zero and positive values are
> free.
> 
> A few drivers just check if the value is non-zero and then return that
> value, but there are just a handful of those.

I don't like messing with that. It can be done because all driver opens go 
through
either v4l2-dev.c or v4l2-subdev.c and we can convert a return value of >0 to 
0. We
have to do that since fs/open.c doesn't check for >0, just != 0.

But that makes our 'open' implementation non-standard, and I feel that that's
both confusing and increases the chances of future bugs (precisely because it is
unexpected).

In pretty much all open() implementations of drivers you will have a 'int err;'
variable or something similar, so being able to do:

        if (v4l2_fh_open_is_first(file, &err)) {
        }

is actually quite efficient (see patch 7/7).

Regards,

        Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to