[v5] telemetry: fix json output buffer size
Checks
Commit Message
Fix json output buffer size for an actual data length.
Fixes: 52af6ccb2b39 ("telemetry: add utility functions for creating JSON")
Signed-off-by: Gowrishankar Muthukrishnan <gmuthukrishn@marvell.com>
---
lib/telemetry/telemetry_json.h | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
Comments
29/09/2021 06:18, Gowrishankar Muthukrishnan:
> Fix json output buffer size for an actual data length.
>
> Fixes: 52af6ccb2b39 ("telemetry: add utility functions for creating JSON")
Please could you give a bit more explanations?
What was not working and why?
[...]
> - * This function is not for use for values larger than 1k.
> + * This function is not for use for values larger than given buffer length.
> */
> __rte_format_printf(3, 4)
> static inline int
> __json_snprintf(char *buf, const int len, const char *format, ...)
> {
> - char tmp[1024];
> + char tmp[len];
> va_list ap;
> int ret;
Hi Thomas,
> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Wednesday, October 6, 2021 11:09 PM
> To: Gowrishankar Muthukrishnan <gmuthukrishn@marvell.com>
> Cc: dev@dpdk.org; bruce.richardson@intel.com; ciara.power@intel.com; Jerin
> Jacob Kollanukkaran <jerinj@marvell.com>
> Subject: [EXT] Re: [dpdk-dev] [v5] telemetry: fix json output buffer size
>
> External Email
>
> ----------------------------------------------------------------------
> 29/09/2021 06:18, Gowrishankar Muthukrishnan:
> > Fix json output buffer size for an actual data length.
> >
> > Fixes: 52af6ccb2b39 ("telemetry: add utility functions for creating
> > JSON")
>
> Please could you give a bit more explanations?
> What was not working and why?
Without this patch, our driver end point (crypto/cnxk) could not successfully deliver the requested info
due to its larger amount than the fixed buffer length of 1024 bytes as handled by __json_snprintf.
I think it is genuine bug too which we caught up here.
Thanks,
Gowrishankar
>
> [...]
> > - * This function is not for use for values larger than 1k.
> > + * This function is not for use for values larger than given buffer length.
> > */
> > __rte_format_printf(3, 4)
> > static inline int
> > __json_snprintf(char *buf, const int len, const char *format, ...) {
> > - char tmp[1024];
> > + char tmp[len];
> > va_list ap;
> > int ret;
>
>
>
07/10/2021 06:58, Gowrishankar Muthukrishnan:
> From: Thomas Monjalon <thomas@monjalon.net>
> > 29/09/2021 06:18, Gowrishankar Muthukrishnan:
> > > Fix json output buffer size for an actual data length.
> > >
> > > Fixes: 52af6ccb2b39 ("telemetry: add utility functions for creating
> > > JSON")
> >
> > Please could you give a bit more explanations?
> > What was not working and why?
>
> Without this patch, our driver end point (crypto/cnxk) could not successfully deliver the requested info
> due to its larger amount than the fixed buffer length of 1024 bytes as handled by __json_snprintf.
> I think it is genuine bug too which we caught up here.
So the commit log should say the JSON message was limited to 1024,
and now you allow any specified length.
> > [...]
> > > - * This function is not for use for values larger than 1k.
> > > + * This function is not for use for values larger than given buffer length.
> > > */
> > > __rte_format_printf(3, 4)
> > > static inline int
> > > __json_snprintf(char *buf, const int len, const char *format, ...) {
> > > - char tmp[1024];
> > > + char tmp[len];
> > > va_list ap;
> > > int ret;
> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Thursday, October 7, 2021 12:52 PM
> To: Gowrishankar Muthukrishnan <gmuthukrishn@marvell.com>
> Cc: dev@dpdk.org; bruce.richardson@intel.com; ciara.power@intel.com; Jerin
> Jacob Kollanukkaran <jerinj@marvell.com>
> Subject: Re: [EXT] Re: [dpdk-dev] [v5] telemetry: fix json output buffer size
>
> 07/10/2021 06:58, Gowrishankar Muthukrishnan:
> > From: Thomas Monjalon <thomas@monjalon.net>
> > > 29/09/2021 06:18, Gowrishankar Muthukrishnan:
> > > > Fix json output buffer size for an actual data length.
> > > >
> > > > Fixes: 52af6ccb2b39 ("telemetry: add utility functions for
> > > > creating
> > > > JSON")
> > >
> > > Please could you give a bit more explanations?
> > > What was not working and why?
> >
> > Without this patch, our driver end point (crypto/cnxk) could not
> > successfully deliver the requested info due to its larger amount than the fixed
> buffer length of 1024 bytes as handled by __json_snprintf.
> > I think it is genuine bug too which we caught up here.
>
> So the commit log should say the JSON message was limited to 1024, and now
> you allow any specified length.
>
Ack. I'll send new version with this correction.
Thanks,
Gowrishankar
> > > [...]
> > > > - * This function is not for use for values larger than 1k.
> > > > + * This function is not for use for values larger than given buffer length.
> > > > */
> > > > __rte_format_printf(3, 4)
> > > > static inline int
> > > > __json_snprintf(char *buf, const int len, const char *format, ...) {
> > > > - char tmp[1024];
> > > > + char tmp[len];
> > > > va_list ap;
> > > > int ret;
>
>
@@ -9,6 +9,7 @@
#include <stdarg.h>
#include <stdio.h>
#include <rte_common.h>
+#include <rte_telemetry.h>
/**
* @file
@@ -23,13 +24,13 @@
* @internal
* Copies a value into a buffer if the buffer has enough available space.
* Nothing written to buffer if an overflow ocurs.
- * This function is not for use for values larger than 1k.
+ * This function is not for use for values larger than given buffer length.
*/
__rte_format_printf(3, 4)
static inline int
__json_snprintf(char *buf, const int len, const char *format, ...)
{
- char tmp[1024];
+ char tmp[len];
va_list ap;
int ret;