Re: [PATCH] dell-wmi: Stop storing pointers to DMI tables

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, 18 Jan 2016 10:09:46 -0800, Andy Lutomirski wrote:
> On Mon, Jan 18, 2016 at 7:44 AM, Jean Delvare <jdelvare@xxxxxxx> wrote:
> > On Sun,  3 Jan 2016 06:52:28 -0800, Andy Lutomirski wrote:
> >> +     if (results->err || results->keymap)
> >> +             return;         /* We already found the hotkey table. */
> >
> > Can this actually happen?
> 
> Yes, I think, if Dell ships a laptop with two tables of type 0xB2.
> There's no return code that says "I'm done", so I can't just stop
> walking the DMI data after finding what I'm looking for.

This may be another thing to consider when redesigning dmi_walk. Maybe
we should let the callback function notify when processing should stop.
If nothing else it should make things slightly faster by avoiding
callbacks for the remaining entries. And it may allow for cleaner
handling of corner cases.

> (...)
> I think the length check is correct, but the hotkey_num calculation is
> wrong.  The table is 84 bytes on my system, which makes perfect sense:
> 6 bytes of header and 78 == 13*6 bytes of entries.  But 84-4 is *not*
> a multiple of 6.

For the record, I don't have a Dell laptop myself but I have a DMI
table dump from a Latitude E6410 and the type 178 record length is 96
bytes (4 + 23 * 4.)

> > (...)
> > I think it would make sense to fix dmi_walk() so that it lets the
> > decoding function return error codes. This would avoid the
> > convoluted error code handling. Not sure why I didn't do that
> > originally :(
> 
> I think that would make sense as a followup.  It'll probably have to
> change the callback's signature, though.

Indeed. That won't be a nice and easy change, but it can still be done.

-- 
Jean Delvare
SUSE L3 Support
--
To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux