On Fri, 05 Aug 2016, ville.syrjala@xxxxxxxxxxxxxxx wrote:
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
> In case the ->debug_output() function skips decoding the register it
> just returns, which means the caller will reuse whatever it already has
> in the tmp buffer as the decoded result for this result. What it usually
> has in there is the decoded result of some previous register.
>
> Showing incorrect decoded results is no good, so let's allow
> ->debug_output() to actually return how many bytes it wrote, and the
> caller can then skip showing the decoded results if zero bytes
> were produced.
>
> We'll make a variant of snprintf() that's safe to call without having to
> check the return value for the case when it didn't have enough space to
> do its work, that is, make it return 0 in case no bytes were written.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> ---
> tools/intel_reg_decode.c | 195 ++++++++++++++++++++++++++---------------------
> 1 file changed, 109 insertions(+), 86 deletions(-)
>
> diff --git a/tools/intel_reg_decode.c b/tools/intel_reg_decode.c
> index 71f3ead6177f..0dbabe81ca3d 100644
> --- a/tools/intel_reg_decode.c
> +++ b/tools/intel_reg_decode.c
> @@ -26,6 +26,7 @@
> */
>
> #include <errno.h>
> +#include <stdarg.h>
> #include <stdbool.h>
> #include <stdint.h>
> #include <stdio.h>
> @@ -39,12 +40,32 @@
> #include "intel_reg_spec.h"
>
> #define _DEBUGSTRING(func) \
> - void func(char *result, int len, int reg, uint32_t val, uint32_t devid)
> + int func(char *result, int len, int reg, uint32_t val, uint32_t devid)
> #define DEBUGSTRING(func) static _DEBUGSTRING(func)
This could use a brief comment describing the return value.
[snip]
> @@ -2631,13 +2652,15 @@ int intel_reg_spec_decode(char *buf, size_t bufsize, const struct reg *reg,
> if (reg->addr != r->reg)
> continue;
>
> - if (r->debug_output)
> - r->debug_output(tmp, sizeof(tmp), r->reg,
> - val, devid);
> - else if (devid)
> + if (r->debug_output) {
> + if (r->debug_output(tmp, sizeof(tmp), r->reg,
> + val, devid) == 0)
> + continue;
> + } else if (devid) {
> return 0;
> - else
> + } else {
> continue;
> + }
Bah, I've included too much trickery between the devid == 0 vs != 0
cases to make this clear. No fault of this patch.
Reviewed-by: Jani Nikula <jani.nikula@xxxxxxxxx>
No need to resend, please just add the comment on the return value.
>
> if (devid) {
> strncpy(buf, tmp, bufsize);
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
