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

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20160121083559.GM7192@pali>
Date:	Thu, 21 Jan 2016 09:35:59 +0100
From:	Pali Rohár <pali.rohar@...il.com>
To:	Michał Kępień <kernel@...pniu.pl>
Cc:	Darren Hart <dvhart@...radead.org>,
	Matthew Garrett <mjg59@...f.ucam.org>,
	Richard Purdie <rpurdie@...ys.net>,
	Jacek Anaszewski <j.anaszewski@...sung.com>,
	Alex Hung <alex.hung@...onical.com>,
	platform-driver-x86@...r.kernel.org, linux-leds@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 01/14] dell-laptop: extract SMBIOS-related code to a
 separate module

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@...il.com