linux-kernel - Re: [PATCH] input: imon driver for SoundGraph iMON/Antec Veris IR devices

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]

Message-Id: <F1D74CA4-2BD9-43C4-8A8F-08B690A334F3@wilsonet.com>
Date:	Wed, 30 Dec 2009 14:01:18 -0500
From:	Jarod Wilson <jarod@...sonet.com>
To:	Dmitry Torokhov <dmitry.torokhov@...il.com>
Cc:	Jarod Wilson <jarod@...hat.com>, linux-input@...r.kernel.org,
	linux-media@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] input: imon driver for SoundGraph iMON/Antec Veris IR devices

On Dec 30, 2009, at 3:02 AM, Jarod Wilson wrote:

> On Dec 29, 2009, at 5:30 PM, Dmitry Torokhov wrote:
> 
>> On Tue, Dec 29, 2009 at 12:04:00AM -0500, Jarod Wilson wrote:
>>> On Dec 28, 2009, at 4:31 AM, Dmitry Torokhov wrote:
>>>> 
>>>> Hm, will this work on big-endian?
>>> 
>>> Good question. Not sure offhand. Probably not. Unfortunately, the only devices I have to test with at the moment are integrated into cases with x86 boards in them, so testing isn't particularly straight-forward. I should probably get ahold of one of the plain external usb devices to play with... Mind if I just add a TODO marker near that for now?
>>> 
>> 
>> How about just do le32_to_cpu() instead of memcpy()ing and that's
>> probably it?
> 
> Hrm. My endian-fu sucks. Not sure what the right way to do it without the memcpy is. I thought I had something together than would work, using 2 lines (memcpy, then le32_to_cpu), but I just realized that'll go south on the keys where we're only looking at half the buffer (i.e., the wrong half on big-endian)... Will see what I can do to fix that up in the morning, was hoping to get this out tonight, but its nearly 3am (again)...

Got something that works in place now. Well, definitely works on x86, works in theory on big-endian, haven't got a way to test it on the latter just yet.


>> [...] I'd still want to see larger functions split up a bit though.
> 
> I've hacked imon_probe() up into 6 different functions now:
> 
> imon_probe()
> -> imon_init_intf0()
>  -> imon_init_idev()
>  -> imon_init_display()
> -> imon_init_intf1()
>  -> imon_init_touch()
> 
> (and there was an existing imon_set_ir_protocol() in the mix)
> 
> Quite a bit more manageable and readable now, I think. Haven't looked yet for other candidates for similar refactoring. Were there others you had in mind? At a glance, imon_incoming_packet() seems to be the only remaining function that is particularly hefty. Well, imon_probe() is still ~180 lines, but it used to be north of 400, I think... So perhaps I try trimming imon_probe() a bit more, and see what can be done to make imon_incoming_packet() less chunky.

Making good headway on imon_incoming_packet(), in-progress bits pushed again. Gotta take my son and a buddy to a movie. Life keeps getting in the way of me finishing this as quickly as I'd like... :)

-- 
Jarod Wilson
jarod@...sonet.com



--
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/