* [PATCH] staging: greybus: split gb_audio_gb_get_topology() into helper functions
@ 2025-12-21 19:42 Khaled Saleh
2025-12-22 6:23 ` Greg KH
2026-01-05 9:29 ` Dan Carpenter
0 siblings, 2 replies; 4+ messages in thread
From: Khaled Saleh @ 2025-12-21 19:42 UTC (permalink / raw)
To: gregkh; +Cc: linux-staging, greybus-dev, khaled
From: khaled <khaled.saleh.req@gmail.com>
The gb_audio_gb_get_topology() function performs multiple distinct
operations: retrieving the topology size, allocating the topology buffer,
and fetching the topology data. A TODO comment in the file notes that
this logic should be split into separate calls.
Introduce helper functions for each step and update
gb_audio_gb_get_topology() to use them.
This improves readability and maintainability without changing behavior.
Signed-off-by: Khaled Saleh <khaled.saleh.req@gmail.com>
---
drivers/staging/greybus/audio_gb.c | 47 +++++++++++++++++++++++-------
1 file changed, 36 insertions(+), 11 deletions(-)
diff --git a/drivers/staging/greybus/audio_gb.c b/drivers/staging/greybus/audio_gb.c
index 9d8994fdb41a..0d5109ec14f6 100644
--- a/drivers/staging/greybus/audio_gb.c
+++ b/drivers/staging/greybus/audio_gb.c
@@ -9,36 +9,61 @@
#include "audio_codec.h"
/* TODO: Split into separate calls */
+static int gb_audio_topology_get_size(struct gb_connection *connection, u16 *size)
+{
+ struct gb_audio_get_topology_size_response resp;
+ int ret;
+
+ ret = gb_operation_sync(connection,
+ GB_AUDIO_TYPE_GET_TOPOLOGY_SIZE,
+ NULL, 0,
+ &resp, sizeof(resp));
+ if (ret)
+ return ret;
+
+ *size = le16_to_cpu(resp.size);
+
+ return 0;
+}
+
+static struct gb_audio_topology *gb_audio_topology_alloc(u16 size)
+{
+ if (size < sizeof(struct gb_audio_topology))
+ return NULL;
+
+ return kzalloc(size, GFP_KERNEL);
+}
+
+static int gb_audio_topology_fetch(struct gb_connection *connection,
+ struct gb_audio_topology *topo, u16 size)
+{
+ return gb_operation_sync(connection,
+ GB_AUDIO_TYPE_GET_TOPOLOGY,
+ NULL, 0,
+ topo, size);
+}
int gb_audio_gb_get_topology(struct gb_connection *connection,
struct gb_audio_topology **topology)
{
- struct gb_audio_get_topology_size_response size_resp;
struct gb_audio_topology *topo;
u16 size;
int ret;
- ret = gb_operation_sync(connection, GB_AUDIO_TYPE_GET_TOPOLOGY_SIZE,
- NULL, 0, &size_resp, sizeof(size_resp));
+ ret = gb_audio_topology_get_size(connection, &size);
if (ret)
return ret;
- size = le16_to_cpu(size_resp.size);
- if (size < sizeof(*topo))
- return -ENODATA;
-
- topo = kzalloc(size, GFP_KERNEL);
+ topo = gb_audio_topology_alloc(size);
if (!topo)
return -ENOMEM;
- ret = gb_operation_sync(connection, GB_AUDIO_TYPE_GET_TOPOLOGY, NULL, 0,
- topo, size);
+ ret = gb_audio_topology_fetch(connection, topo, size);
if (ret) {
kfree(topo);
return ret;
}
*topology = topo;
-
return 0;
}
EXPORT_SYMBOL_GPL(gb_audio_gb_get_topology);
--
2.43.0
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] staging: greybus: split gb_audio_gb_get_topology() into helper functions
2025-12-21 19:42 [PATCH] staging: greybus: split gb_audio_gb_get_topology() into helper functions Khaled Saleh
@ 2025-12-22 6:23 ` Greg KH
2026-01-05 9:29 ` Dan Carpenter
1 sibling, 0 replies; 4+ messages in thread
From: Greg KH @ 2025-12-22 6:23 UTC (permalink / raw)
To: Khaled Saleh; +Cc: linux-staging, greybus-dev
On Sun, Dec 21, 2025 at 09:42:00PM +0200, Khaled Saleh wrote:
> From: khaled <khaled.saleh.req@gmail.com>
This, does not match:
> Signed-off-by: Khaled Saleh <khaled.saleh.req@gmail.com>
this.
Please fix up for your next submission of this change.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] staging: greybus: split gb_audio_gb_get_topology() into helper functions
2025-12-21 19:42 [PATCH] staging: greybus: split gb_audio_gb_get_topology() into helper functions Khaled Saleh
2025-12-22 6:23 ` Greg KH
@ 2026-01-05 9:29 ` Dan Carpenter
1 sibling, 0 replies; 4+ messages in thread
From: Dan Carpenter @ 2026-01-05 9:29 UTC (permalink / raw)
To: Khaled Saleh; +Cc: gregkh, linux-staging, greybus-dev
On Sun, Dec 21, 2025 at 09:42:00PM +0200, Khaled Saleh wrote:
> From: khaled <khaled.saleh.req@gmail.com>
Full name, please.
>
> The gb_audio_gb_get_topology() function performs multiple distinct
> operations: retrieving the topology size, allocating the topology buffer,
> and fetching the topology data. A TODO comment in the file notes that
> this logic should be split into separate calls.
>
> Introduce helper functions for each step and update
> gb_audio_gb_get_topology() to use them.
>
> This improves readability and maintainability without changing behavior.
>
> Signed-off-by: Khaled Saleh <khaled.saleh.req@gmail.com>
> ---
> drivers/staging/greybus/audio_gb.c | 47 +++++++++++++++++++++++-------
> 1 file changed, 36 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/staging/greybus/audio_gb.c b/drivers/staging/greybus/audio_gb.c
> index 9d8994fdb41a..0d5109ec14f6 100644
> --- a/drivers/staging/greybus/audio_gb.c
> +++ b/drivers/staging/greybus/audio_gb.c
> @@ -9,36 +9,61 @@
> #include "audio_codec.h"
>
> /* TODO: Split into separate calls */
> +static int gb_audio_topology_get_size(struct gb_connection *connection, u16 *size)
Use size_t for sizes.
> +{
> + struct gb_audio_get_topology_size_response resp;
> + int ret;
> +
> + ret = gb_operation_sync(connection,
> + GB_AUDIO_TYPE_GET_TOPOLOGY_SIZE,
> + NULL, 0,
> + &resp, sizeof(resp));
> + if (ret)
> + return ret;
> +
> + *size = le16_to_cpu(resp.size);
> +
> + return 0;
> +}
> +
> +static struct gb_audio_topology *gb_audio_topology_alloc(u16 size)
Same.
> +{
> + if (size < sizeof(struct gb_audio_topology))
> + return NULL;
> +
> + return kzalloc(size, GFP_KERNEL);
> +}
> +
> +static int gb_audio_topology_fetch(struct gb_connection *connection,
> + struct gb_audio_topology *topo, u16 size)
Same.
It's fine for now since there is just one caller, but using anything
other that size_t for sizes is just asking for integer overflow bugs.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH] staging: greybus: split gb_audio_gb_get_topology() into helper functions
@ 2025-12-22 16:15 Khaled Saleh
0 siblings, 0 replies; 4+ messages in thread
From: Khaled Saleh @ 2025-12-22 16:15 UTC (permalink / raw)
To: gregkh; +Cc: linux-staging, greybus-dev, Khaled Saleh
The gb_audio_gb_get_topology() function performs multiple distinct
operations: retrieving the topology size, allocating the topology buffer,
and fetching the topology data. A TODO comment in the file notes that
this logic should be split into separate calls.
Introduce helper functions for each step and update
gb_audio_gb_get_topology() to use them.
This improves readability and maintainability without changing behavior.
Signed-off-by: Khaled Saleh <khaled.saleh.req@gmail.com>
---
drivers/staging/greybus/audio_gb.c | 47 +++++++++++++++++++++++-------
1 file changed, 36 insertions(+), 11 deletions(-)
diff --git a/drivers/staging/greybus/audio_gb.c b/drivers/staging/greybus/audio_gb.c
index 9d8994fdb41a..0d5109ec14f6 100644
--- a/drivers/staging/greybus/audio_gb.c
+++ b/drivers/staging/greybus/audio_gb.c
@@ -9,36 +9,61 @@
#include "audio_codec.h"
/* TODO: Split into separate calls */
+static int gb_audio_topology_get_size(struct gb_connection *connection, u16 *size)
+{
+ struct gb_audio_get_topology_size_response resp;
+ int ret;
+
+ ret = gb_operation_sync(connection,
+ GB_AUDIO_TYPE_GET_TOPOLOGY_SIZE,
+ NULL, 0,
+ &resp, sizeof(resp));
+ if (ret)
+ return ret;
+
+ *size = le16_to_cpu(resp.size);
+
+ return 0;
+}
+
+static struct gb_audio_topology *gb_audio_topology_alloc(u16 size)
+{
+ if (size < sizeof(struct gb_audio_topology))
+ return NULL;
+
+ return kzalloc(size, GFP_KERNEL);
+}
+
+static int gb_audio_topology_fetch(struct gb_connection *connection,
+ struct gb_audio_topology *topo, u16 size)
+{
+ return gb_operation_sync(connection,
+ GB_AUDIO_TYPE_GET_TOPOLOGY,
+ NULL, 0,
+ topo, size);
+}
int gb_audio_gb_get_topology(struct gb_connection *connection,
struct gb_audio_topology **topology)
{
- struct gb_audio_get_topology_size_response size_resp;
struct gb_audio_topology *topo;
u16 size;
int ret;
- ret = gb_operation_sync(connection, GB_AUDIO_TYPE_GET_TOPOLOGY_SIZE,
- NULL, 0, &size_resp, sizeof(size_resp));
+ ret = gb_audio_topology_get_size(connection, &size);
if (ret)
return ret;
- size = le16_to_cpu(size_resp.size);
- if (size < sizeof(*topo))
- return -ENODATA;
-
- topo = kzalloc(size, GFP_KERNEL);
+ topo = gb_audio_topology_alloc(size);
if (!topo)
return -ENOMEM;
- ret = gb_operation_sync(connection, GB_AUDIO_TYPE_GET_TOPOLOGY, NULL, 0,
- topo, size);
+ ret = gb_audio_topology_fetch(connection, topo, size);
if (ret) {
kfree(topo);
return ret;
}
*topology = topo;
-
return 0;
}
EXPORT_SYMBOL_GPL(gb_audio_gb_get_topology);
--
2.43.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-01-05 9:29 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-21 19:42 [PATCH] staging: greybus: split gb_audio_gb_get_topology() into helper functions Khaled Saleh
2025-12-22 6:23 ` Greg KH
2026-01-05 9:29 ` Dan Carpenter
-- strict thread matches above, loose matches on Subject: below --
2025-12-22 16:15 Khaled Saleh
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox