[PATCH 2/2] i2c-tools: fix build with upstream i2c-tools' headers

[Bartosz Gołaszewski]

2017-08-28 15:52 GMT+02:00 Bartosz Gołaszewski <bartekgola at gmail.com>:
> 2017-08-28 15:48 GMT+02:00 Denys Vlasenko <vda.linux at googlemail.com>:
>> On Mon, Aug 28, 2017 at 11:49 AM, Bartosz Golaszewski
>> <bartekgola at gmail.com> wrote:
>>> Installing headers from upstream i2c-tools makes it impossible to
>>> build the busybox version due to redefined symbols. This is caused by
>>> the fact that linux/i2c-dev.h from i2c-tools overwrites the one from
>>> linux uapi and defines symbols already defined by linux' linux/i2c.h.
>>>
>>> Fix it by including linux/i2c.h and defining the i2c access routines
>>> only if a well known symbol provided by linux uapi is not defined in
>>> linux/i2c-dev.h.
>>>
>>> Tested by building on systems with and without i2c-tools headers.
>>>
>>> Signed-off-by: Bartosz Golaszewski <bartekgola at gmail.com>
>>> ---
>>>  miscutils/i2c_tools.c | 16 +++++++++++++++-
>>>  1 file changed, 15 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/miscutils/i2c_tools.c b/miscutils/i2c_tools.c
>>> index 03a09d519..9cc55eb27 100644
>>> --- a/miscutils/i2c_tools.c
>>> +++ b/miscutils/i2c_tools.c
>>> @@ -62,7 +62,6 @@
>>>
>>>  #include "libbb.h"
>>>
>>> -#include <linux/i2c.h>
>>>  #include <linux/i2c-dev.h>
>>>
>>>  #define I2CDUMP_NUM_REGS               256
>>> @@ -79,6 +78,19 @@ static ALWAYS_INLINE void *itoptr(int i)
>>>         return (void*)(intptr_t)i;
>>>  }
>>>
>>> +/*
>>> + * Only include linux/i2c.h if the symbols provided by it are not already
>>> + * defined in linux/i2c-dev.h. This - while not correct - happens if we
>>> + * install the headers provided by upstream i2c-tools and is the default on
>>> + * some distributions (Debian, OpenSUSE). linux/i2c-dev.h from i2c-tools
>>> + * overwrites the linux uapi header and redefines a bunch of symbols, as
>>> + * well as already provides the i2c access functions we redefine below.
>>
>> Oh boy. They have "static inlines" there, right?
>> That would add some hundreds of bytes of code.
>>
>> Maybe we should drop "#include <linux/i2c-dev.h>" instead?
>>
>> What are we using from those, "struct i2c_smbus_ioctl_data"
>> and "struct i2c_rdwr_ioctl_data" only?
>
> Just removing this include is giving me this:
>
>   CC      miscutils/i2c_tools.o
> miscutils/i2c_tools.c: In function ‘i2c_smbus_access’:
> miscutils/i2c_tools.c:86:30: error: storage size of ‘args’ isn’t known
>   struct i2c_smbus_ioctl_data args;
>                               ^~~~
> miscutils/i2c_tools.c:93:19: error: ‘I2C_SMBUS’ undeclared (first use
> in this function)
>   return ioctl(fd, I2C_SMBUS, &args);
>                    ^~~~~~~~~
> miscutils/i2c_tools.c:93:19: note: each undeclared identifier is
> reported only once for each function it appears in
> miscutils/i2c_tools.c:86:30: warning: unused variable ‘args’ [-Wunused-variable]
>   struct i2c_smbus_ioctl_data args;
>                               ^~~~
> miscutils/i2c_tools.c: In function ‘i2c_set_pec’:
> miscutils/i2c_tools.c:258:30: error: ‘I2C_PEC’ undeclared (first use
> in this function)
>   ioctl_or_perror_and_die(fd, I2C_PEC,
>                               ^~~~~~~
> miscutils/i2c_tools.c: In function ‘i2c_set_slave_addr’:
> miscutils/i2c_tools.c:265:38: error: ‘I2C_SLAVE_FORCE’ undeclared
> (first use in this function)
>   ioctl_or_perror_and_die(fd, force ? I2C_SLAVE_FORCE : I2C_SLAVE,
>                                       ^~~~~~~~~~~~~~~
> miscutils/i2c_tools.c:265:56: error: ‘I2C_SLAVE’ undeclared (first use
> in this function)
>   ioctl_or_perror_and_die(fd, force ? I2C_SLAVE_FORCE : I2C_SLAVE,
>                                                         ^~~~~~~~~
> miscutils/i2c_tools.c: In function ‘get_funcs_matrix’:
> miscutils/i2c_tools.c:307:30: error: ‘I2C_FUNCS’ undeclared (first use
> in this function)
>   ioctl_or_perror_and_die(fd, I2C_FUNCS, funcs,
>                               ^~~~~~~~~
> miscutils/i2c_tools.c: In function ‘i2cdetect_main’:
> miscutils/i2c_tools.c:1304:23: error: ‘I2C_SLAVE’ undeclared (first
> use in this function)
>     status = ioctl(fd, I2C_SLAVE, itoptr(i + j));
>                        ^~~~~~~~~
> miscutils/i2c_tools.c: In function ‘i2c_smbus_access’:
> miscutils/i2c_tools.c:94:1: warning: control reaches end of non-void
> function [-Wreturn-type]
>  }
>
> I can play a bit with it and see if we can drop this file while not
> re-writing the whole header.
>
> --
> Best regards,
> Bartosz Golaszewski

It turns out only a couple defines and a single struct were needed to
drop linux/i2c-dev.h. Please take a look at the new patch.

-- 
Best regards,
Bartosz Golaszewski