lkml.org 
[lkml]   [2019]   [Dec]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v2 3/3] leds: trigger: implement a tty trigger
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/ |

\
 
 \ /
  Last update: 2019-12-17 11:59    [W:0.051 / U:15.580 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site