linux-kernel - Re: [PATCH 0/11] introduce macros for i2c

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20121009173237.7c1a49e9@endymion.delvare>
Date:	Tue, 9 Oct 2012 17:32:37 +0200
From:	Jean Delvare <khali@...ux-fr.org>
To:	Julia Lawall <Julia.Lawall@...6.fr>
Cc:	Mauro Carvalho Chehab <mchehab@...radead.org>, ben-linux@...ff.org,
	w.sang@...gutronix.de, linux-i2c@...r.kernel.org,
	kernel-janitors@...r.kernel.org, rmallon@...il.com,
	shubhrajyoti@...com, linux-media@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 0/11] introduce macros for i2c_msg initialization

Hi Julia,

On Sun,  7 Oct 2012 17:38:30 +0200, Julia Lawall wrote:
> This patch set introduces some macros for describing how an i2c_msg is
> being initialized.  There are three macros: I2C_MSG_READ, for a read
> message, I2C_MSG_WRITE, for a write message, and I2C_MSG_OP, for some other
> kind of message, which is expected to be very rarely used.

"Some other kind of message" is actually messages which need extra
flags. They are still read or write messages.

OK, I've read the whole series now and grepped the kernel tree so I
have a better overview. There are a lot more occurrences than what you
converted. I presume the conversions were just an example and you leave
the rest up to the relevant maintainers (e.g. me) if they are
interested?

Given the huge number of affected drivers (a quick grep suggests 230
drivers and more than 300 occurrences), we'd better think twice before
going on as it will be intrusive and hard to change afterward.

So my first question will be: what is your goal with this change? Are
you only trying to save a few lines of source code? Or do you expect to
actually fix/prevent bugs by introducing these macros? Or something
else?

I admit I am not completely convinced by the benefit at the moment. A
number of these drivers should be using i2c_smbus_*() functions instead
of i2c_transfer() for improved compatibility, or i2c_master_send/recv()
for single message transfers (383 occurrences!), so making
i2c_transfer() easier to use isn't at the top of my priority list. And
I see the extra work for the pre-processor, so we need a good reason
for doing that.

-- 
Jean Delvare
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/