linux-kernel - Re: [PATCH] usb: gadget: f_fs: simplify ffs

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: <8737elz9ji.fsf@linux.intel.com>
Date:   Fri, 10 Mar 2017 12:54:41 +0200
From:   Felipe Balbi <felipe.balbi@...ux.intel.com>
To:     Michal Nazarewicz <mina86@...a86.com>
Cc:     linux-kernel@...r.kernel.org, linux-usb@...r.kernel.org,
        Michal Nazarewicz <mina86@...a86.com>
Subject: Re: [PATCH] usb: gadget: f_fs: simplify ffs_dev name handling


Hi,

Michal Nazarewicz <mina86@...a86.com> writes:
> Currently ffs_dev::name can be either allocated by the client of
> the ffs_dev structure or by the f_fs.c core itself.  The former
> is used by g_ffs while the latter happens with configfs.
>
> Historically, g_ffs did not need to allocate separate buffer for
> the name so what is now f_fs.c core never cared about freeing
> that space.  With configfs the name needs to be copied since the
> memory is not guaranteed to be availeble after ffs_set_inst_name
> finishes.
>
> The complication is therefore here to avoid allocations in the
> g_ffs case but it complicates the code inproportinally to
> benefits it provides.  In particular, g_ffs is considered
> ‘legacy’ so optimising for its sake is unlikely to be worth the
> effort.
>
> With that observation in mind, simplify the code by unifying the
> code paths in g_ffs and configfs paths.  Furthermore, instead of
> allocating a new buffer for the name, simply embed it in the
> ffs_dev structure.  This further makes the memory management
> less convoluted and error-prone.
>
> The configfs interface for functionfs imposed a limit of 40
> characters for the name so this results in a 41-byte buffer
> added to the structure.  (For short names this may lead to
> wasted memory but the actual amount is not immediately obvious
> and depends on pointer size and which slab buckets the structure
> and name would fall into).
>
> Signed-off-by: Michal Nazarewicz <mina86@...a86.com>
> ---
>  drivers/usb/gadget/function/f_fs.c | 79 ++++++++------------------------------
>  drivers/usb/gadget/function/u_fs.h | 11 +++---
>  2 files changed, 22 insertions(+), 68 deletions(-)
>
> diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c
> index f5d6bf527aac..6ed4da6e4474 100644
> --- a/drivers/usb/gadget/function/f_fs.c
> +++ b/drivers/usb/gadget/function/f_fs.c
> @@ -245,7 +245,6 @@ EXPORT_SYMBOL_GPL(ffs_lock);
>  
>  static struct ffs_dev *_ffs_find_dev(const char *name);
>  static struct ffs_dev *_ffs_alloc_dev(void);
> -static int _ffs_name_dev(struct ffs_dev *dev, const char *name);
>  static void _ffs_free_dev(struct ffs_dev *dev);
>  static void *ffs_acquire_dev(const char *dev_name);
>  static void ffs_release_dev(struct ffs_data *ffs_data);
> @@ -3277,13 +3276,13 @@ static LIST_HEAD(ffs_devices);
>  
>  static struct ffs_dev *_ffs_do_find_dev(const char *name)
>  {
> -	struct ffs_dev *dev;
> +	if (name) {

how about avoiding the extra indentation level?

	if (!name)
        	return NULL;

	[...]

-- 
balbi

Download attachment "signature.asc" of type "application/pgp-signature" (833 bytes)