linux-kernel - Re: [RFC] i2c-tools: i2ctransfer: add new tool

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] [day] [month] [year] [list]

Date:	Sat, 9 May 2015 09:09:14 +0200
From:	Wolfram Sang <wsa@...-dreams.de>
To:	Jean Delvare <jdelvare@...e.de>
Cc:	linux-i2c@...r.kernel.org, linux-sh@...r.kernel.org,
	Magnus Damm <magnus.damm@...il.com>,
	Simon Horman <horms@...ge.net.au>,
	Laurent Pinchart <laurent.pinchart@...asonboard.com>,
	Geert Uytterhoeven <geert@...ux-m68k.org>,
	linux-kernel@...r.kernel.org
Subject: Re: [RFC] i2c-tools: i2ctransfer: add new tool

> BTW I'm curious if you actually needed the + and - suffixes in practice?
> I can easily imagine how the = suffix will be useful in the real
> world, but + and -...  Or maybe just for bus driver testing purpose?

Exactly for that. While you can send complex messages with my tool, it
should be clear this is mainly for testing/developing. Once you know
what you need to do to access a slave, a specfic driver is the better
option, be it in userspace or kernel space.

Those patterns are easily recognizable when fed into an EEPROM. They can
already reveal quite some problems with bus drivers, e.g. off-by-one
errors when fetching data, sending STOP etc.

> > +}
> > +	/* let Linux free malloced memory on termination */
> 
> I don't like this. The memory allocated in i2cbusses.c is freed
> explicitly, so it is inconsistent to not free yours. Freeing the memory

Well, it is documented :)

> explicitly makes the code easier to read and debug as it documents the

I disagree about this one. Currently, we have a LOT of error paths when
parsing input fails. Getting all the error paths right with freeing
memory is usually error-prone (especially with later additions) and will
IMO make the code way less readable. Since the lifetime of those buffers
ends right before the exit(0), it seems unnecessarily complex to me.

However, I need to redo the parsing anyhow. Let's see how it looks like
in the next version. If it is easy/readable to free(), I'll do it.

Download attachment "signature.asc" of type "application/pgp-signature" (820 bytes)