Em 27-06-2011 11:56, Hans Verkuil escreveu: > On Monday, June 27, 2011 15:54:11 Mauro Carvalho Chehab wrote: >> Em 27-06-2011 09:17, Hans Verkuil escreveu: >>> While we don't have an enum capability, in many cases you can deduce >>> whether a particular ioctl should be supported or not. Usually based on >>> capabilities, sometimes because certain ioctls allow 'NOP' operations that >>> allow you to test for their presence. >>> >>> Of course, drivers are not always consistent here, but that's a separate >>> problem. >> >> Any "hint" code that would try to do some NOP operations may fail. One of the >> reasons is that such hint is not documented. Yet, I don't officially support >> such "hint" methods at the API. > > The point is that the spec can easily be improved to make such 'NOP' > operations > explicit, or to require that if a capability is present, then the > corresponding > ioctl(s) must also be present. Things like that are easy to verify as well > with > v4l2-compliance.
We currently have more than 64 ioctl's. Adding a capability bit for each doesn't seem the right thing to do. Ok, some could be grouped, but, even so, there are drivers that implement the VIDIOC_G, but doesn't implement the corresponding VIDIO_S. So, I think we don't have enough available bits for doing that. >> Btw, there are two drivers returning -ENOTTY, when the device got >> disconnected >> (or firmware were not uploaded). >> >> The truth is that the current API specs for return code is bogus. > > Bogus in what way? It's been documented very clearly for years. We may not > like > that design decision (I certainly don't like it), but someone clearly thought > about it at the time. Bogus in the sense that drivers don't follow them, as they're returning undocumented values. Any application strictly following it will have troubles. >> The right thing to do is to create a separate chapter for error codes, based >> on errno(3) >> man page, where we document all error codes that should be used by the >> drivers. Then, >> at the ioctl pages, link to the common chapter and, only when needed, >> document special >> cases where an error code for that specific ioctl has some special meaning. > > Great, I've no problem with that. But this particular error code you want to > change > is actually implemented *consistently* in all drivers. There is no confusion, > no > ambiguity, and it is according to the spec. As I said, from userspace perspective, it is not consistent to assume that EINVAL means not implemented. For sure at VIDIOC_S_foo, this is not consistent. Even on some GET types of ioctl, like for example [1][2], there are other reasons for an EINVAL return. [1] http://linuxtv.org/downloads/v4l-dvb-apis/vidioc-cropcap.html [2] http://linuxtv.org/downloads/v4l-dvb-apis/vidioc-g-audio.html The only way to make it consistent is to use different return codes for "invalid parameters" and for "unsupported ioctl". >> I ran a script here to check how many different error codes are used inside >> drivers/media: >> >> $ find drivers/media -type f -name '*.[ch]' >files >> $ grep define `find . -name errno*.h`|perl -ne 'print "$1\n" if >> (/\#define\s+(E[^\s]+)/)'|sort|uniq >errors >> $ for i in `cat errors`; do COUNT=$(git grep -c $i `cat files`|wc -l); if [ >> "$COUNT" != "0" ]; then echo $i $COUNT; fi; done >> >> The result is that we're using 53 different types of errors, but the API >> specs documents >> only 17 of them. Those are the currently used errors at drivers/media: >> >> ERROR CODE |NUMBER OF *.c/*.h FILES USING IT >> ---------------|-------------------------------- >> E2BIG 1 >> EACCES 8 >> EAGAIN 66 >> EBADF 1 >> EBADFD 1 >> EBADR 2 >> EBADRQC 2 >> EBUSY 149 >> ECHILD 1 >> ECONNRESET 25 >> EDEADLK 1 >> EDOM 1 >> EEXIST 3 >> EFAULT 230 >> EFBIG 1 >> EILSEQ 8 >> EINIT 2 >> EINPROGRESS 6 >> EINTR 21 >> EINVAL 501 >> EIO 305 >> EMFILE 1 >> ENFILE 7 >> ENOBUFS 7 >> ENODATA 4 >> ENODEV 270 >> ENOENT 46 >> ENOIOCTLCMD 31 >> ENOMEM 359 >> ENOSPC 13 >> ENOSR 7 >> ENOSYS 15 >> ENOTSUP 3 >> ENOTSUPP 3 >> ENOTTY 5 >> ENXIO 26 >> EOPNOTSUPP 19 >> EOVERFLOW 14 >> EPERM 47 >> EPIPE 12 >> EPROTO 11 >> ERANGE 25 >> EREMOTE 80 >> EREMOTEIO 80 >> ERESTART 32 >> ERESTARTSYS 32 >> ESHUTDOWN 27 >> ESPIPE 3 >> ETIME 53 >> ETIMEDOUT 37 >> EUSERS 2 >> EWOULDBLOCK 14 >> EXDEV 1 >> >> I suspect that we'll need to both fix some drivers, and the API, as I bet >> that >> the same error conditions are reported differently on different drivers. >> >>> I don't think changing such an important return value is acceptable. >> >> As I said, the current API is bogus with respect to error codes. Of course, >> we need to do take care to avoid userspace applications breakage, but we >> can't >> use the excuse that it is there for a long time as a reason for not fixing >> it. > > The fact that many drivers use error codes creatively doesn't give us an > excuse > to just change the one error code that is actually used everywhere according > to > the spec! That's faulty logic. The fix that it is needed is to provide a consistent way for an userspace application to know for sure when an ioctl is not supported. It can be done on a simple way of just returning a different error code for it, or with complex mechanisms like adding a per-ioctl flag and some hint logics based on NOP. The V4L2 is complex enough for us to add more complexity with hints and cap flags. Thanks, Mauro -- 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
