Re: [PATCH 6/6] DVB API: LNA documentation

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 09/12/2012 03:45 PM, Mauro Carvalho Chehab wrote:
Em 12-09-2012 08:01, David Waring escreveu:
On 11/09/12 19:38, Mauro Carvalho Chehab wrote:
Em 16-08-2012 22:35, Antti Palosaari escreveu:
[snip]
+	<para>Possible values: 0, 1, INT_MIN</para>

Hmm... INT_MIN... are you sure it is portable on all Linux compilers?

I don't like the idea on trusting on whatever C/C++/Java/... compiler (or some interpreter)
would define as "INT_MIN".

The better is to define a value for that, or, instead, to define something
at the API header file that won't cause troubles with 32 bits or 64 bits
userspace, like defining it as:

#define DVB_AUTO_LNA ((u32)~0)

INT_MIN is defined in limits.h which is an ISO standard header. Other
parts of the kernel also use INT_MIN, e.g. linux/cpu.h and
linux/netfilter_ipv4.h both reference INT_MIN from limits.h.

The linux/cpu.h is a Kernel internal header. There's no public userspace API
there. So, it uses kernel's own definition for INT_MIN.

You're right with regards to netfilter. Btw, it is only places where INT_MIN
is used on an userspace-filtered headers are at the netfilter interface :

/usr/include/linux/netfilter_ipv6.h:#include <limits.h> /* for INT_MIN, INT_MAX */
/usr/include/linux/netfilter_ipv6.h:	NF_IP6_PRI_FIRST = INT_MIN,
/usr/include/linux/netfilter_ipv4.h:#include <limits.h> /* for INT_MIN, INT_MAX */
/usr/include/linux/netfilter_ipv4.h:	NF_IP_PRI_FIRST = INT_MIN,
/usr/include/linux/netfilter_decnet.h:#include <limits.h> /* for INT_MIN, INT_MAX */
/usr/include/linux/netfilter_decnet.h:	NF_DN_PRI_FIRST = INT_MIN,

Even so, it got renamed inside a priorities enum:

enum nf_ip_hook_priorities {
	NF_IP_PRI_FIRST = INT_MIN,

In the case of netfilter, as this is just a priority number, it actually
make sense to use INT_MIN as the lowest priority, as INT_MIN is the lowest
number that can be represented there.

What we're doing here is something else: we're defining a special value to be
interpreted as "AUTO". In this case, if Kernel and userspace disagrees on what
value should be used, the KAPI will be deadly broken.

I might be wrong, but some C compilers on a few architectures (Tru64 C compiler comes
on my mind) define "int" as 64 bit integers, with will affect the definition of INT_MIN.
Ok, in this case, the definition will be compatible, but I'm wondering if some other
compiler might be doing something else here.

That's why I'm in favor of defining some constant for "AUTO" at the kernel headers,
in a way that we'll be sure that we won't have any bad surprises on userspace.

Could you say clearly what it should be as I am not very familiar with API changes?

Also you mentioned with multistream support API changes that those values are unsigned numbers => not negative which makes me more unsure.
http://www.mail-archive.com/linux-media@xxxxxxxxxxxxxxx/msg50772.html

I can say I tested it with Python script using value -2147483648 and it worked. Maybe it was still due to signed => unsigned conversion.

Antti

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


[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux