From mboxrd@z Thu Jan 1 00:00:00 1970 From: Laurent Pinchart Subject: Re: [RFCv2 PATCH 4/9] v4l: add buffer exporting via dmabuf Date: Tue, 27 Mar 2012 12:21:27 +0200 Message-ID: <1361191.IRCd6g5UzF@avalon> References: <1331633827-503-1-git-send-email-t.stanislaws@samsung.com> <2646632.PEZlYAVAnk@avalon> <4F6C5F84.2070100@samsung.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [95.142.166.194]) by gabe.freedesktop.org (Postfix) with ESMTP id E810F9E75C for ; Tue, 27 Mar 2012 03:20:54 -0700 (PDT) In-Reply-To: <4F6C5F84.2070100@samsung.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org Errors-To: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org To: Tomasz Stanislawski Cc: ben@bwidawsk.net, daniel.vetter@ffwll.ch, dri-devel@lists.freedesktop.org, linaro-mm-sig@lists.linaro.org, kyungmin.park@samsung.com, sakari.ailus@iki.fi, airlied@redhat.com, m.szyprowski@samsung.com List-Id: dri-devel@lists.freedesktop.org Hi Tomasz, On Friday 23 March 2012 12:33:24 Tomasz Stanislawski wrote: > On 03/22/2012 12:16 PM, Laurent Pinchart wrote: > > On Tuesday 13 March 2012 11:17:02 Tomasz Stanislawski wrote: [snip] > >> + case VIDIOC_EXPBUF: > >> + { > >> + struct v4l2_exportbuffer *p = arg; > >> + > >> + if (!ops->vidioc_expbuf) > >> + break; > >> + > >> + ret = ops->vidioc_expbuf(file, fh, p); > > > > You can pass arg to ops->vidioc_expbuf() directly, there's no need to > > create a struct v4l2_exportbuffer *p variable. > > No problem. I tried to follow style of other ioctls. > Notice that adding this temporary variable provides some form of type > checking. I mean using a proper structure for a proper callback. It makes sure that the argument passed to video_expbuf is indeed a v4l2_exportbuffer, but it doesn't check that the arg pointer you assign to p points to the right type, so it's a bit pointless in my opinion. This construct makes sense if you need to access field of the arg pointer here before or after calling the operation. > >> + break; > >> + } -- Regards, Laurent Pinchart