diff mbox series

[1/3] bitrate: change reg implementation to match API description

Message ID 20210709151938.701895-1-ktraynor@redhat.com (mailing list archive)
State New
Delegated to: Thomas Monjalon
Headers show
Series [1/3] bitrate: change reg implementation to match API description | expand

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Kevin Traynor July 9, 2021, 3:19 p.m. UTC
rte_stats_bitrate_reg() API states it returns 'Zero on success'.

However, the implementation directly returns the return of
rte_metrics_reg_names() which may be zero or positive on success,
with a positive value also indicating the index.

The user of rte_stats_bitrate_reg() should not care about the
index as it is stored in the opaque rte_stats_bitrates struct.

Change the implementation of rte_stats_bitrate_reg() to match
the API description by always returning zero on success.

Fixes: 2ad7ba9a6567 ("bitrate: add bitrate statistics library")

Signed-off-by: Kevin Traynor <ktraynor@redhat.com>
---
 lib/bitratestats/rte_bitrate.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Thomas Monjalon July 22, 2021, 7:46 p.m. UTC | #1
09/07/2021 17:19, Kevin Traynor:
> rte_stats_bitrate_reg() API states it returns 'Zero on success'.
> 
> However, the implementation directly returns the return of
> rte_metrics_reg_names() which may be zero or positive on success,
> with a positive value also indicating the index.
> 
> The user of rte_stats_bitrate_reg() should not care about the
> index as it is stored in the opaque rte_stats_bitrates struct.
> 
> Change the implementation of rte_stats_bitrate_reg() to match
> the API description by always returning zero on success.
> 
> Fixes: 2ad7ba9a6567 ("bitrate: add bitrate statistics library")
> 
> Signed-off-by: Kevin Traynor <ktraynor@redhat.com>

Does it require a deprecation notice?
At least I suggest a release note in API section.

What is the target for this series? 21.11?
Kevin Traynor July 22, 2021, 8:24 p.m. UTC | #2
On 22/07/2021 20:46, Thomas Monjalon wrote:
> 09/07/2021 17:19, Kevin Traynor:
>> rte_stats_bitrate_reg() API states it returns 'Zero on success'.
>>
>> However, the implementation directly returns the return of
>> rte_metrics_reg_names() which may be zero or positive on success,
>> with a positive value also indicating the index.
>>
>> The user of rte_stats_bitrate_reg() should not care about the
>> index as it is stored in the opaque rte_stats_bitrates struct.
>>
>> Change the implementation of rte_stats_bitrate_reg() to match
>> the API description by always returning zero on success.
>>
>> Fixes: 2ad7ba9a6567 ("bitrate: add bitrate statistics library")
>>
>> Signed-off-by: Kevin Traynor <ktraynor@redhat.com>
> 
> Does it require a deprecation notice?

I'm not certain, but I don't think it does. It is fixing the
implementation so it behaves as the API is documented to.

> At least I suggest a release note in API section.
> 
> What is the target for this series? 21.11?
> 

No urgency, 21.11 is fine for this set.

> 
>
diff mbox series

Patch

diff --git a/lib/bitratestats/rte_bitrate.c b/lib/bitratestats/rte_bitrate.c
index 8fd9f47288..e23e38bc94 100644
--- a/lib/bitratestats/rte_bitrate.c
+++ b/lib/bitratestats/rte_bitrate.c
@@ -56,6 +56,8 @@  rte_stats_bitrate_reg(struct rte_stats_bitrates *bitrate_data)
 
 	return_value = rte_metrics_reg_names(&names[0], RTE_DIM(names));
-	if (return_value >= 0)
+	if (return_value >= 0) {
 		bitrate_data->id_stats_set = return_value;
+		return 0;
+	}
 	return return_value;
 }