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

[Pali RohÃr]

On Wednesday 20 January 2016 10:21:07 MichaÅ KÄpieÅ wrote:
> > > +extern struct calling_interface_buffer *buffer;
> > > +extern struct calling_interface_token *da_tokens;
> > 
> > Better hide this variable in dell-smbios.c code ...
> > 
> > > +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.
> 
> Before I spam everyone with another set of 15 patches, I'd like to
> discuss this a bit further.  There is no point in passing the buffer to
> release_buffer(), because it only unlocks a mutex.  I also see no point
> in passing the buffer to clear_buffer() and dell_send_request(), because
> there is always just one buffer to operate on.
> 
> A total of four functions have something to do with the SMBIOS buffer:
> 
>   * get_buffer()
>   * clear_buffer()
>   * release_buffer()
>   * dell_send_request()
> 
> This rework is a chance to make them all consistent, i.e. remove the
> SMBIOS buffer from their argument lists.  This way we can "signal" this
> API's users that there is only one SMBIOS buffer ever involved while
> still removing the extern and EXPORT_SYMBOL_GPL for the buffer.  BTW, I
> also see little point in returning the buffer from dell_send_request()
> as none of its callers in dell-laptop assign its return value to
> anything (i.e. there is no "buffer = dell_send_request(buffer, ...)" in
> the code).
> 
> To sum up, I'd suggest that function prototypes could look like this:
> 
>     struct calling_interface_buffer *dell_smbios_get_buffer(void);
>     void dell_smbios_clear_buffer(void);
>     void dell_smbios_release_buffer(void);
>     void dell_smbios_send_request(int class, int select);
> 
> What do you think?
> 

In other scenario functions should do something like this:

struct buf *buf_alloc(void);
buf_clear(struct buf *buf);
buf_free(struct buf *buf);
buf_do_something(struct buf *buf, ...);

But here I do not know how hard is to create alloc/free functions and
what is cost for creating that buffer in first 4GB memory...

-- 
Pali RohÃr
pali.rohar@xxxxxxxxx