Messages in this thread | | | Date | Mon, 18 Jan 2016 11:34:32 +0100 | From | Michał Kępień <> | Subject | Re: [PATCH 01/14] dell-laptop: extract SMBIOS-related code to a separate module |
| |
Hi Pali,
Thanks for taking a look.
> > +struct calling_interface_token { > > + u16 tokenID; > > + u16 location; > > + union { > > + u16 value; > > + u16 stringlength; > > + }; > > +}; > > After patch 12/14 you do not need to define this struct in header file.
dell_smbios_find_token() returns a struct calling_interface_token *, which can then be dereferenced by the caller. See patch 13.
> > +extern struct calling_interface_buffer *buffer; > > +extern struct calling_interface_token *da_tokens; > > Better hide this variable in dell-smbios.c code ...
Patch 12 makes da_tokens static, but I believe you were referring to buffer.
> > +void clear_buffer(void); > > +void get_buffer(void); > > +void release_buffer(void); > > ... and let those functions to get parameter to buffer. > > E.g. get_buffer will return buffer and other two functions will take > buffer parameter.
You're right. My original approach looked tempting because it reduces the amount of changes required in dell-laptop, but on second thought, it _assumes_ that users of this API would play nicely with the SMBIOS buffer while your way _enforces_ it.
I will prepare a v2 including this suggestion within the next couple of days.
-- Best regards, Michał Kępień
| |