[Drbd-dev] [PATCH 0/3] lru_cache: Adjustments for lc_create()

[Drbd-dev] [PATCH 0/3] lru_cache: Adjustments for lc_create()

[Markus Elfring]

Date: Wed, 29 Mar 2023 15:30:23 +0200

Some update suggestions were taken into account
from static source code analysis.

Markus Elfring (3):
  Return directly after a failed kzalloc()
  Improve two size determinations
  Improve exception handling

 lib/lru_cache.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

--
2.40.0

[Drbd-dev] [PATCH 1/3] lru_cache: Return directly after a failed kzalloc() in lc_create()

[Markus Elfring]

Date: Wed, 29 Mar 2023 14:45:34 +0200

1. Return directly after a call of the function “kzalloc” failed
   at the beginning in these function implementations.

2. Omit extra initialisations (for the variables “slot” and “element”)
   which became unnecessary with this refactoring.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 lib/lru_cache.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/lib/lru_cache.c b/lib/lru_cache.c
index b3d9187611de..e0db27b3a2d7 100644
--- a/lib/lru_cache.c
+++ b/lib/lru_cache.c
@@ -78,8 +78,8 @@ struct lru_cache *lc_create(const char *name, struct kmem_cache *cache,
 		unsigned max_pending_changes,
 		unsigned e_count, size_t e_size, size_t e_off)
 {
-	struct hlist_head *slot = NULL;
-	struct lc_element **element = NULL;
+	struct hlist_head *slot;
+	struct lc_element **element;
 	struct lru_cache *lc;
 	struct lc_element *e;
 	unsigned cache_obj_size = kmem_cache_size(cache);
@@ -96,7 +96,8 @@ struct lru_cache *lc_create(const char *name, struct kmem_cache *cache,

 	slot = kcalloc(e_count, sizeof(struct hlist_head), GFP_KERNEL);
 	if (!slot)
-		goto out_fail;
+		return NULL;
+
 	element = kcalloc(e_count, sizeof(struct lc_element *), GFP_KERNEL);
 	if (!element)
 		goto out_fail;
--
2.40.0

[Drbd-dev] [PATCH 2/3] lru_cache: Improve two size determinations in lc_create()

[Markus Elfring]

Date: Wed, 29 Mar 2023 15:00:13 +0200

Replace the specification of data structures by pointer dereferences
as the parameter for the operator “sizeof” to make the corresponding size
determination a bit safer according to the Linux coding style convention.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 lib/lru_cache.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/lru_cache.c b/lib/lru_cache.c
index e0db27b3a2d7..31820f03b146 100644
--- a/lib/lru_cache.c
+++ b/lib/lru_cache.c
@@ -94,11 +94,11 @@ struct lru_cache *lc_create(const char *name, struct kmem_cache *cache,
 	if (e_count > LC_MAX_ACTIVE)
 		return NULL;

-	slot = kcalloc(e_count, sizeof(struct hlist_head), GFP_KERNEL);
+	slot = kcalloc(e_count, sizeof(*slot), GFP_KERNEL);
 	if (!slot)
 		return NULL;

-	element = kcalloc(e_count, sizeof(struct lc_element *), GFP_KERNEL);
+	element = kcalloc(e_count, sizeof(*element), GFP_KERNEL);
 	if (!element)
 		goto out_fail;

--
2.40.0

[Drbd-dev] [PATCH 3/3] lru_cache: Improve exception handling in lc_create()

[Markus Elfring]

Date: Wed, 29 Mar 2023 15:20:39 +0200

The label “out_fail” was used to jump to a kfree() call despite of
the detail in the implementation of the function “lc_create”
that it was determined already that a corresponding variable contained
a null pointer because of a failed memory allocation.

Thus use more appropriate labels instead.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 lib/lru_cache.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/lib/lru_cache.c b/lib/lru_cache.c
index 31820f03b146..fdc8bd6fc888 100644
--- a/lib/lru_cache.c
+++ b/lib/lru_cache.c
@@ -100,11 +100,11 @@ struct lru_cache *lc_create(const char *name, struct kmem_cache *cache,

 	element = kcalloc(e_count, sizeof(*element), GFP_KERNEL);
 	if (!element)
-		goto out_fail;
+		goto free_slot;

 	lc = kzalloc(sizeof(*lc), GFP_KERNEL);
 	if (!lc)
-		goto out_fail;
+		goto free_element;

 	INIT_LIST_HEAD(&lc->in_use);
 	INIT_LIST_HEAD(&lc->lru);
@@ -142,8 +142,9 @@ struct lru_cache *lc_create(const char *name, struct kmem_cache *cache,
 		kmem_cache_free(cache, p - e_off);
 	}
 	kfree(lc);
-out_fail:
+free_element:
 	kfree(element);
+free_slot:
 	kfree(slot);
 	return NULL;
 }
--
2.40.0

Re: [Drbd-dev] [PATCH 0/3] lru_cache: Adjustments for lc_create()

[Markus Elfring]

> Date: Wed, 29 Mar 2023 15:30:23 +0200
>
> Some update suggestions were taken into account
> from static source code analysis.
>
> Markus Elfring (3):
>   Return directly after a failed kzalloc()
>   Improve two size determinations
>   Improve exception handling
>
>  lib/lru_cache.c | 18 ++++++++++--------
>  1 file changed, 10 insertions(+), 8 deletions(-)


Is this patch series still in review queues?

See also:
https://lore.kernel.org/cocci/33226beb-4fe2-3da5-5d69-a33e683dec57@web.de/
https://sympa.inria.fr/sympa/arc/cocci/2023-03/msg00110.html

Regards,
Markus