Hello Greg,
On Tue, Dec 17, 2019 at 09:32:11AM +0100, Greg Kroah-Hartman wrote:
> On Tue, Dec 17, 2019 at 09:17:18AM +0100, Uwe Kleine-König wrote:
> > + ret = tty_get_icount(trigger_data->tty, &icount);
> > + if (icount.rx > trigger_data->icount.rx ||
> > + icount.tx > trigger_data->icount.tx) {
>
> What happens when icount.rx and/or icount.tx wraps? It's "only" an int.
Good catch. I wonder why this is not an unsigned quantity. Just grepping
through drivers/tty/serial most drivers just increment these counters
and don't care for overflow (which is undefined for ints) either. :-\
..ooOO(Where is the can maintainer? --- We found a can of worms :-)
> > + unsigned long delay_on = 100, delay_off = 100;
> > +
> > + led_blink_set_oneshot(trigger_data->led_cdev,
> > + &delay_on, &delay_off, 0);
> > +
> > + trigger_data->icount = icount;
>
> Implicit memcpy of a structure? Ick.
I'd call that elegant ;-)
> All you care about are the two integers, why not just track them instead
> of the whole thing?
For now I only care about tx and rx, but I intend to add some bells and
whistles to trigger on other events. (But I don't care much, can add
that once I implement this support.)
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
