From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from mx1.redhat.com ([209.132.183.28]:57003 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753101Ab1GGRqV (ORCPT ); Thu, 7 Jul 2011 13:46:21 -0400 Message-ID: <4E15F0EA.4080003@redhat.com> Date: Thu, 07 Jul 2011 14:46:18 -0300 From: Mauro Carvalho Chehab MIME-Version: 1.0 To: Hans Verkuil CC: Linux Media Mailing List , Linux Kernel Mailing List Subject: Re: [PATCH RFCv3 03/17] [media] DocBook: Use the generic error code page also for MC API References: <201107071729.03676.hverkuil@xs4all.nl> <4E15E9AC.9050800@redhat.com> <201107071928.57090.hverkuil@xs4all.nl> In-Reply-To: <201107071928.57090.hverkuil@xs4all.nl> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit List-ID: Sender: Em 07-07-2011 14:28, Hans Verkuil escreveu: > On Thursday, July 07, 2011 19:15:24 Mauro Carvalho Chehab wrote: >> Em 07-07-2011 12:29, Hans Verkuil escreveu: >>> On Wednesday, July 06, 2011 20:03:52 Mauro Carvalho Chehab wrote: >>>> Instead of having their own generic error codes at the MC API, move >>>> its section to the generic one and be sure that all media ioctl's >>>> will point to it. >>>> >>>> Signed-off-by: Mauro Carvalho Chehab >>>> >>>> diff --git a/Documentation/DocBook/media/v4l/gen-errors.xml b/Documentation/DocBook/media/v4l/gen-errors.xml >>>> index 6ef476a..a7f73c9 100644 >>>> --- a/Documentation/DocBook/media/v4l/gen-errors.xml >>>> +++ b/Documentation/DocBook/media/v4l/gen-errors.xml >>>> @@ -5,6 +5,11 @@ >>>> >>>> &cs-str; >>>> >>>> + >>>> + >>>> + EBADF >>>> + fd is not a valid open file descriptor. >>>> + >>>> >>>> EBUSY >>>> The ioctl can't be handled because the device is busy. This is >>>> @@ -15,7 +20,16 @@ >>>> problem first (typically: stop the stream before retrying). >>>> >>>> >>>> + EFAULT >>>> + fd is not a valid open file descriptor. >>> >>> This seems to be a copy-and-paste error. The original text in media-func-ioctl.xml says this: >>> >>> argp references an inaccessible memory >>> area. >> >> Ah, yes. Anyway, a latter patch changes it to: > > OK, I missed that. It was a bit confusing to review. Yeah. Documentation patches are harder to handle than normal patches. I did several changes on the existing patches, but perfecting each patch individually will probably take forever. >> >> EFAULT >> There was a failure while copying data from/to userspace. >> >> >> referencing a parameter name there is a bad thing anyway, as this is now at the common >> ioctl error code. >> >> Instead of just using a posix-like error code: >> EFAULT Bad address (POSIX.1) >> >> I opted to use a more valuable description, explaining the reason for such error, >> e. g. that there was a failure at the data copy from/to userspace. >> >> It may be better to change it to: >> >> EFAULT >> There was a failure while copying data from/to userspace, probably >> caused by an invalid pointer reference. >> >> I think I'll add the above description at the latter patch. >> >> I was intending to add there the other possible error causes found at V4L/DVB API's >> and drivers, but the changes I did took me a longer time than I was expecting >> originally. I'll eventually do that when I have more time. >> >> It would be really great if we could find some volunteer to help syncing >> the media API specs with the code. >> >>>> + >>>> + >>>> EINVAL >>>> + One or more of the ioctl parameters are invalid. This is a widely >>> >>> widely -> widely used >>> >>>> + error code. see the individual ioctl requests for actual causes. >>> >>> see -> See >> >> Fixed. > > OK, with these changes you have my > > Acked-by: Hans Verkuil > > as well for this patch. Thanks! > Just for my understanding: do you plan on merging this for 3.1? I have no objection to > that. Together with the querycap version changes it is easy to add compatibility support > to libv4l should that be necessary. I'm not convinced there won't be any fallout from > this change, but at least there is a decent way of working around it if needed. And there > is no doubt that -ENOTTY is a much better return code. Yes, that's my plan. Having both patch series merged together seemed a good idea to me, as it becomes easier for applications to benefit of that. > > When this is merged I'll modify v4l2-compliance, v4l2-ctl (if necessary) and qv4l2. OK. > > Regards, > > Hans