Hello, On Mon, Mar 13, 2017 at 12:01:40PM +0100, Wolfram Sang wrote: > Hi Uwe, > > thanks for the review! > > > > +.RI [ data ] > > > +.RI [ desc > > > +.RI [ data ]] > > > > You could join the previous two lines. > > Try it. You will miss some spaces, then. It works fine with quoting: .RI [ "desc " [ data "]] ..." . But ok, it's at least arguable if this is better than your solution. > > > +Also, you cannot be interrupted by another I2C master during one transfer, but it might happen between multiple transfers. > > > > Well, unless you loose arbitration. Maybe this is too picky to be > > relevant here? > > I wonder: will another I2C master start a transfer on a repeated start? > Need to investigate. I'd say it must not. It should have logic to detect bus busy and delay any transfers until the bus becomes idle. With a repeated start the bus doesn't become idle between two transfers. > > > + for (i = 0; i <= nmsgs; i++) > > > + free(msgs[i].buf); > > > + > > > + exit(1); > > > > return EXIT_FAILURE; ? > > > > Apart from the exit code this is exactly the trailer of the good path, > > so these could share code. > > No! One has '< nmsgs', the other one '<= nmsgs'. Friendly rant: It was > all easier and less subtle before Jean wanted the 'don't rely on the OS > for cleanup' additions ;) ah, ok. BTW, I like tools that clean up after themselves. This way debugging is much easier if you look for lost memory. And yes, this doesn't matter much for short-living programs like i2c-tools, but I like being consistent here and also do the cleanup for this this type of program. Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | http://www.pengutronix.de/ |
