Re: [PATCH 01/14] dell-laptop: extract SMBIOS-related code to a separate module

From: MichaÅ KÄpieÅ
Date: Mon Jan 18 2016 - 05:34:42 EST


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Å