Messages in this thread | | | Date | Thu, 26 Nov 2020 22:31:33 +0800 | From | Leo Yan <> | Subject | Re: [PATCH v9 01/16] perf arm-spe: Refactor printing string to buffer |
| |
On Thu, Nov 26, 2020 at 09:11:58AM -0300, Arnaldo Carvalho de Melo wrote: > Em Thu, Nov 19, 2020 at 11:24:26PM +0800, Leo Yan escreveu: > > When outputs strings to the decoding buffer with function snprintf(), > > SPE decoder needs to detects if any error returns from snprintf() and if > > so needs to directly bail out. If snprintf() returns success, it needs > > to update buffer pointer and reduce the buffer length so can continue to > > output the next string into the consequent memory space. > > > > This complex logics are spreading in the function arm_spe_pkt_desc() so > > there has many duplicate codes for handling error detecting, increment > > buffer pointer and decrement buffer size. > > > > To avoid the duplicate code, this patch introduces a new helper function > > arm_spe_pkt_out_string() which is used to wrap up the complex logics, > > and it's used by the caller arm_spe_pkt_desc(). This patch moves the > > variable 'blen' as the function's local variable so allows to remove > > the unnecessary braces and improve the readability. > > > > This patch simplifies the return value for arm_spe_pkt_desc(): '0' means > > success and other values mean an error has occurred. To realize this, > > it relies on arm_spe_pkt_out_string()'s parameter 'err', the 'err' is a > > cumulative value, returns its final value if printing buffer is called > > for one time or multiple times. Finally, the error is handled in a > > central place, rather than directly bailing out in switch-cases, it > > returns error at the end of arm_spe_pkt_desc(). > > > > This patch changes the caller arm_spe_dump() to respect the updated > > return value semantics of arm_spe_pkt_desc(). > > > > Suggested-by: Dave Martin <Dave.Martin@arm.com> > > Signed-off-by: Leo Yan <leo.yan@linaro.org> > > Reviewed-by: Andre Przywara <andre.przywara@arm.com> > > Reviewed-by: Dave Martin <Dave.Martin@arm.com> > > --- > > .../arm-spe-decoder/arm-spe-pkt-decoder.c | 302 +++++++++--------- > > tools/perf/util/arm-spe.c | 2 +- > > 2 files changed, 151 insertions(+), 153 deletions(-) > > > > diff --git a/tools/perf/util/arm-spe-decoder/arm-spe-pkt-decoder.c b/tools/perf/util/arm-spe-decoder/arm-spe-pkt-decoder.c > > index 671a4763fb47..fbededc1bcd4 100644 > > --- a/tools/perf/util/arm-spe-decoder/arm-spe-pkt-decoder.c > > +++ b/tools/perf/util/arm-spe-decoder/arm-spe-pkt-decoder.c > > @@ -9,6 +9,7 @@ > > #include <endian.h> > > #include <byteswap.h> > > #include <linux/bitops.h> > > +#include <stdarg.h> > > > > #include "arm-spe-pkt-decoder.h" > > > > @@ -258,192 +259,189 @@ int arm_spe_get_packet(const unsigned char *buf, size_t len, > > return ret; > > } > > > > +static int arm_spe_pkt_out_string(int *err, char **buf_p, size_t *blen, > > + const char *fmt, ...) > > +{ > > + va_list ap; > > + int ret; > > > Ok, from a quick look this continues confusing, but at least its not > named scnprintf() and then people won't expect the usual semantics.
Shame for the confusion :(
> Applying.
Thanks, Arnaldo.
| |