perf tools: Update strbuf to remove xrealloc
v3 cover perf tools: Update strbuf to remove xrealloc
Hi Arnaldo,
Here is the 3rd version of the strbuf update. I've fixed it according to your suggestion :)
- [1/8] Use realloc instead of malloc/memcpy/free (Thanks Arnaldo!)
- [2/8] Cleanup code path according to Arnaldo's review.
- [5/8] Avoid using STRBUF_INIT and return error directly before initalizing strbuf.
This patch series does refactoring strbuf and xrealloc related code to remove xrealloc since it can call die() to exit immediately when it hits any error. Instead of that, it should return error code to the caller so that the caller can handle its error.
Thus, at first, this changes the strbuf APIs to return error code instead of die() immediately. And then changing API callers according to the following rules.
- Check the return value of strbuf APIs and handle errors and,
- If the caller returns an error code (errno), it returns the return value of strbuf APIs.
- If the caller just return -1 or NULl in error case, it also returns -1 or NULL in case of strbuf error.
- If the caller can call die() directly, it also call die() in case of strbuf error.
- Error checking patches are splitted for each subcommand, since it will help review.
This also removes xrealloc and ALLOC_GROW from libperf, so that no one use it anymore.
Thank you,
---
Masami Hiramatsu (8):
perf: Rewrite strbuf not to die
perf probe: Check the return value of strbuf APIs
perf help: Make check_emacsclient_version to check strbuf APIs
perf: Make alias handler to check return value of strbuf perf header: Make topology checkers to check return value of strbuf perf pmu: Make pmu_formats_string to check return value of strbuf perf help: Do not use ALLOC_GROW in add_cmd_list
perf tools: Remove xrealloc and ALLOC_GROW
tools/perf/builtin-help.c | 18 +++-- tools/perf/perf.c | 8 +- tools/perf/util/Build | 1 tools/perf/util/cache.h | 19 ----- tools/perf/util/dwarf-aux.c | 52 ++++++------- tools/perf/util/header.c | 31 +++++--- tools/perf/util/help-unknown-cmd.c | 30 ++++++-- tools/perf/util/pmu.c | 10 +-- tools/perf/util/probe-event.c | 143 ++++++++++++++++++++++-------------- tools/perf/util/probe-finder.c | 30 +++++--- tools/perf/util/quote.c | 36 +++++---- tools/perf/util/quote.h | 2 - tools/perf/util/strbuf.c | 93 +++++++++++++++++------ tools/perf/util/strbuf.h | 25 ++++-- tools/perf/util/util.h | 6 -- tools/perf/util/wrapper.c | 29 ------- 16 files changed, 293 insertions(+), 240 deletions(-) delete mode 100644 tools/perf/util/wrapper.c
--
Masami Hiramatsu
PATCH v3 1/8 perf: Rewrite strbuf not to die
Rewrite strbuf implementation not to use die() nor xrealloc(). Instead of die, now most of the API returns error code or 0 if succeeded.
---
Changes in v3:
- Use realloc instead of malloc/memcpy/free (Thanks Arnaldo!)
--- tools/perf/util/strbuf.c | 93 +++++++++++++++++++++++++++++++++------------- tools/perf/util/strbuf.h | 25 +++++++----- 2 files changed, 82 insertions(+), 36 deletions(-)
tools/perf/util/strbuf.c +67 -26
@@ -1,3 +1,4 @@ +#include "debug.h" #include "cache.h" #include <linux/kernel.h>
@@ -17,12 +18,13 @@ int prefixcmp(const char *str, const char *prefix)*/ char strbuf_slopbuf[1 ]; -void strbuf_init (struct strbuf *sb,ssize_t hint) +int strbuf_init (struct strbuf *sb,ssize_t hint) { sb->alloc = sb->len =0 ; sb->buf = strbuf_slopbuf;if (hint) -strbuf_grow (sb, hint); +return strbuf_grow (sb, hint); +return 0 ; }void strbuf_release (struct strbuf *sb)
@@ -42,67 +44,104 @@ char *strbuf_detach(struct strbuf *sb, size_t *sz)return res; } -void strbuf_grow (struct strbuf *sb,size_t extra) +int strbuf_grow (struct strbuf *sb,size_t extra) { -if (sb->len + extra +1 <= sb->len) -die ("you want to use way too much memory" ); -if (!sb->alloc) - sb->buf =NULL ; -ALLOC_GROW (sb->buf, sb->len + extra +1 , sb->alloc); +char *buf; +size_t nr = sb->len + extra +1 ; + +if (nr < sb->alloc) +return 0 ; + +if (nr <= sb->len) +return -E2BIG ; + +if (alloc_nr (sb->alloc) > nr) + nr =alloc_nr (sb->alloc); + +/* +* Note that sb->buf == strbuf_slopbuf if sb->alloc == 0, and it is +* a static variable. Thus we have to avoid passing it to realloc. +*/ + buf =realloc (sb->alloc ? sb->buf :NULL , nr *sizeof (*buf)); +if (!buf) +return -ENOMEM ; + + sb->buf = buf; + sb->alloc = nr; +return 0 ; } -void strbuf_addch (struct strbuf *sb,int c) +int strbuf_addch (struct strbuf *sb,int c) { -strbuf_grow (sb,1 ); +int ret =strbuf_grow (sb,1 ); +if (ret) +return ret; + sb->buf[sb->len++] = c; sb->buf[sb->len] ='\0' ; +return 0 ; } -void strbuf_add (struct strbuf *sb,const void *data,size_t len) +int strbuf_add (struct strbuf *sb,const void *data,size_t len) { -strbuf_grow (sb, len); +int ret =strbuf_grow (sb, len); +if (ret) +return ret; +memcpy (sb->buf + sb->len, data, len); -strbuf_setlen (sb, sb->len + len); +return strbuf_setlen (sb, sb->len + len); } -static void strbuf_addv (struct strbuf *sb,const char *fmt,va_list ap) +static int strbuf_addv (struct strbuf *sb,const char *fmt,va_list ap) { -int len; +int len, ret;va_list ap_saved; -if (!strbuf_avail (sb)) -strbuf_grow (sb,64 ); +if (!strbuf_avail (sb)) { + ret =strbuf_grow (sb,64 ); +if (ret) +return ret; + }va_copy (ap_saved, ap); len =vsnprintf (sb->buf + sb->len, sb->alloc - sb->len, fmt, ap);if (len <0 ) -die ("your vsnprintf is broken" ); +return len;if (len >strbuf_avail (sb)) { -strbuf_grow (sb, len); + ret =strbuf_grow (sb, len); +if (ret) +return ret; len =vsnprintf (sb->buf + sb->len, sb->alloc - sb->len, fmt, ap_saved);va_end (ap_saved);if (len >strbuf_avail (sb)) { -die ("this should not happen, your vsnprintf is broken" ); +pr_debug ("this should not happen, your vsnprintf is broken" ); +return -EINVAL ; } } -strbuf_setlen (sb, sb->len + len); +return strbuf_setlen (sb, sb->len + len); } -void strbuf_addf (struct strbuf *sb,const char *fmt, ...) +int strbuf_addf (struct strbuf *sb,const char *fmt, ...) {va_list ap; +int ret;va_start (ap, fmt); -strbuf_addv (sb, fmt, ap); + ret =strbuf_addv (sb, fmt, ap);va_end (ap); +return ret; }ssize_t strbuf_read (struct strbuf *sb,int fd,ssize_t hint) {size_t oldlen = sb->len;size_t oldalloc = sb->alloc; +int ret; + + ret =strbuf_grow (sb, hint ? hint :8192 ); +if (ret) +return ret; -strbuf_grow (sb, hint ? hint :8192 );for (;;) {ssize_t cnt;
@@ -112,12 +151,14 @@ ssize_t strbuf_read(struct strbuf *sb, int fd, ssize_t hint)strbuf_release (sb); elsestrbuf_setlen (sb ,oldlen ); -return -1 ; +return cnt; }if (!cnt)break ; sb->len += cnt; -strbuf_grow (sb,8192 ); + ret =strbuf_grow (sb,8192 ); +if (ret) +return ret; } sb->buf[sb->len] ='\0' ;
tools/perf/util/strbuf.h +15 -10
@@ -51,7 +51,7 @@ struct strbuf {#define STRBUF_INIT { 0, 0, strbuf_slopbuf }/*----- strbuf life cycle -----*/ -void strbuf_init (struct strbuf *buf,ssize_t hint); +int strbuf_init (struct strbuf *buf,ssize_t hint);void strbuf_release (struct strbuf *buf);char *strbuf_detach (struct strbuf *buf,size_t *);
@@ -60,26 +60,31 @@ static inline ssize_t strbuf_avail(const struct strbuf *sb) {return sb->alloc ? sb->alloc - sb->len -1 :0 ; } -void strbuf_grow (struct strbuf *buf,size_t ); +int strbuf_grow (struct strbuf *buf,size_t ); -static inline void strbuf_setlen (struct strbuf *sb,size_t len) { -if (!sb->alloc) -strbuf_grow (sb,0 ); +static inline int strbuf_setlen (struct strbuf *sb,size_t len) { +int ret; +if (!sb->alloc) { + ret =strbuf_grow (sb,0 ); +if (ret) +return ret; + }assert (len < sb->alloc); sb->len = len; sb->buf[len] ='\0' ; +return 0 ; }/*----- add data in your buffer -----*/ -void strbuf_addch (struct strbuf *sb,int c); +int strbuf_addch (struct strbuf *sb,int c); -void strbuf_add (struct strbuf *buf,const void *,size_t ); -static inline void strbuf_addstr (struct strbuf *sb,const char *s) { -strbuf_add (sb, s,strlen (s)); +int strbuf_add (struct strbuf *buf,const void *,size_t ); +static inline int strbuf_addstr (struct strbuf *sb,const char *s) { +return strbuf_add (sb, s,strlen (s)); } __attribute__((format (printf,2 ,3 ))) -void strbuf_addf (struct strbuf *sb,const char *fmt, ...); +int strbuf_addf (struct strbuf *sb,const char *fmt, ...);/* XXX: if read fails, any partial read is undone */ ssize_t strbuf_read (struct strbuf *,int fd,ssize_t hint);
PATCH v3 2/8 perf probe: Check the return value of strbuf APIs
In reply to Masami HiramatsuCheck the return value of strbuf APIs in perf-probe related code, so that it can handle errors in strbuf.
---
Changes in v3:
- Cleanup code path according to Arnaldo's review.
Changes in V2:
- Rebased on the latest perf/core.
--- tools/perf/util/dwarf-aux.c | 52 +++++++-------- tools/perf/util/probe-event.c | 143 ++++++++++++++++++++++++---------------- tools/perf/util/probe-finder.c | 30 +++++--- 3 files changed, 128 insertions(+), 97 deletions(-)
tools/perf/util/dwarf-aux.c +24 -28
@@ -915,8 +915,7 @@ int die_get_typename(Dwarf_Die *vr_die, struct strbuf *buf) tmp ="*" ; else if (tag == DW_TAG_subroutine_type) {/* Function pointer */ -strbuf_add (buf,"(function_type)" ,15 ); -return 0 ; +return strbuf_add (buf,"(function_type)" ,15 ); } else {if (!dwarf_diename (&type))return -ENOENT ;
@@ -927,14 +926,10 @@ int die_get_typename(Dwarf_Die *vr_die, struct strbuf *buf)else if (tag == DW_TAG_enumeration_type) tmp ="enum " ;/* Write a base name */ -strbuf_addf (buf,"%s%s" , tmp,dwarf_diename (&type)); -return 0 ; +return strbuf_addf (buf,"%s%s" , tmp,dwarf_diename (&type)); } ret =die_get_typename (&type, buf); -if (ret ==0 ) -strbuf_addstr (buf, tmp); - -return ret; +return ret ? ret :strbuf_addstr (buf, tmp); }/**
@@ -951,12 +946,10 @@ int die_get_varname(Dwarf_Die *vr_die, struct strbuf *buf) ret =die_get_typename (vr_die, buf);if (ret <0 ) {pr_debug ("Failed to get type, make it unknown.\n" ); -strbuf_add (buf," (unknown_type)" ,14 ); + ret =strbuf_add (buf," (unknown_type)" ,14 ); } -strbuf_addf (buf,"\t%s" ,dwarf_diename (vr_die)); - -return 0 ; +return ret <0 ? ret :strbuf_addf (buf,"\t%s" ,dwarf_diename (vr_die)); }#ifdef HAVE_DWARF_GETLOCATIONS
@@ -999,22 +992,24 @@ static int die_get_var_innermost_scope(Dwarf_Die *sp_die, Dwarf_Die *vr_die, }while ((offset =dwarf_ranges (&scopes[1 ], offset, &base, - &start, &end)) >0 ) { + &start, &end)) >0 ) { start -= entry; end -= entry;if (first) { -strbuf_addf (buf,"@<%s+[%" PRIu64"-%" PRIu64, - name, start, end); + ret =strbuf_addf (buf,"@<%s+[%" PRIu64"-%" PRIu64, + name, start, end); first = false; }else { -strbuf_addf (buf,",%" PRIu64"-%" PRIu64, - start, end); + ret =strbuf_addf (buf,",%" PRIu64"-%" PRIu64, + start, end); } +if (ret <0 ) + goto out; }if (!first) -strbuf_add (buf,"]>" ,2 ); + ret =strbuf_add (buf,"]>" ,2 ); out:free (scopes);
@@ -1054,31 +1049,32 @@ int die_get_var_range(Dwarf_Die *sp_die, Dwarf_Die *vr_die, struct strbuf *buf)if (dwarf_attr (vr_die, DW_AT_location, &attr) ==NULL )return -EINVAL ; -while ((offset =dwarf_getlocations ( - &attr, offset, &base, - &start, &end, &op, &nops)) >0 ) { +while ((offset =dwarf_getlocations (&attr, offset, &base, + &start, &end, &op, &nops)) >0 ) {if (start ==0 ) {/* Single Location Descriptions */ ret =die_get_var_innermost_scope (sp_die, vr_die, buf); -return ret; + goto out; }/* Location Lists */ start -= entry; end -= entry;if (first) { -strbuf_addf (buf,"@<%s+[%" PRIu64"-%" PRIu64, - name, start, end); + ret =strbuf_addf (buf,"@<%s+[%" PRIu64"-%" PRIu64, + name, start, end); first = false; }else { -strbuf_addf (buf,",%" PRIu64"-%" PRIu64, - start, end); + ret =strbuf_addf (buf,",%" PRIu64"-%" PRIu64, + start, end); } +if (ret <0 ) + goto out; }if (!first) -strbuf_add (buf,"]>" ,2 ); - + ret =strbuf_add (buf,"]>" ,2 ); +out: return ret; }#else
tools/perf/util/probe-event.c +86 -57
270 lines not on this page · raw message
tools/perf/util/probe-finder.c +18 -12
@@ -1294,6 +1294,7 @@ static int collect_variables_cb(Dwarf_Die *die_mem, void *data) {struct available_var_finder *af = data;struct variable_list *vl; +struct strbuf buf =STRBUF_INIT ;int tag, ret; vl = &af->vls[af->nvls -1 ];
@@ -1307,25 +1308,26 @@ static int collect_variables_cb(Dwarf_Die *die_mem, void *data)if (ret ==0 || ret == -ERANGE ) {int ret2;bool externs = !af->child; -struct strbuf buf; -strbuf_init (&buf,64 ); +if (strbuf_init (&buf,64 ) <0 ) + goto error;if (probe_conf.show_location_range) { -if (!externs) { -if (ret) -strbuf_add (&buf,"[INV]\t" ,6 ); -else -strbuf_add (&buf,"[VAL]\t" ,6 ); - }else -strbuf_add (&buf,"[EXT]\t" ,6 ); +if (!externs) + ret2 =strbuf_add (&buf, + ret ?"[INV]\t" :"[VAL]\t" ,6 ); +else + ret2 =strbuf_add (&buf,"[EXT]\t" ,6 ); +if (ret2) + goto error; } ret2 =die_get_varname (die_mem, &buf);if (!ret2 && probe_conf.show_location_range && !externs) { -strbuf_addch (&buf,'\t' ); +if (strbuf_addch (&buf,'\t' ) <0 ) + goto error; ret2 =die_get_var_range (&af->pf.sp_die, die_mem, &buf); }
@@ -1334,8 +1336,8 @@ static int collect_variables_cb(Dwarf_Die *die_mem, void *data)if (ret2 ==0 ) {strlist__add (vl->vars,strbuf_detach (&buf,NULL )); - }else -strbuf_release (&buf); + } +strbuf_release (&buf); } }
@@ -1343,6 +1345,10 @@ static int collect_variables_cb(Dwarf_Die *die_mem, void *data)return DIE_FIND_CB_CONTINUE ; else return DIE_FIND_CB_SIBLING; +error: +strbuf_release (&buf); +pr_debug ("Error in strbuf\n" ); +return DIE_FIND_CB_END ; }/* Add a found vars into available variables list */
PATCH v3 3/8 perf help: Make check_emacsclient_version to check strbuf APIs
In reply to Masami HiramatsuMake check_emacsclient_version() to check the return value of strbuf APIs so that it can handle errors in strbuf.
--- tools/perf/builtin-help.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-)
tools/perf/builtin-help.c +10 -8
@@ -61,6 +61,7 @@ static int check_emacsclient_version(void)struct child_process ec_process;const char *argv_ec[] = {"emacsclient" ,"--version" ,NULL };int version; +int ret =-1 ;/* emacsclient prints its version number on stderr */ memset (&ec_process,0 ,sizeof (ec_process));
@@ -71,7 +72,10 @@ static int check_emacsclient_version(void)fprintf (stderr,"Failed to start emacsclient.\n" );return -1 ; } -strbuf_read (&buffer, ec_process.err,20 ); +if (strbuf_read (&buffer, ec_process.err,20 ) <0 ) { +fprintf (stderr,"Failed to read emacsclient version\n" ); + goto out; + }close (ec_process.err);/*
@@ -82,8 +86,7 @@ static int check_emacsclient_version(void)if (prefixcmp (buffer.buf,"emacsclient" )) {fprintf (stderr,"Failed to parse emacsclient version.\n" ); -strbuf_release (&buffer); -return -1 ; + goto out; } version =atoi (buffer.buf +strlen ("emacsclient" ));
@@ -92,12 +95,11 @@ static int check_emacsclient_version(void)fprintf (stderr,"emacsclient version '%d' too old (< 22).\n" , version); -strbuf_release (&buffer); -return -1 ; - } - + }else + ret =0 ; +out:strbuf_release (&buffer); -return 0 ; +return ret; }static void exec_woman_emacs (const char *path,const char *page)
PATCH v3 4/8 perf: Make alias handler to check return value of strbuf
In reply to Masami HiramatsuMake alias handler and sq_quote_argv to check the return value of strbuf APIs.
In sq_quote_argv() calls die(), but this fix handles strbuf failure as a special case and returns to caller, since the caller - handle_alias() also has to check the return value of other strbuf APIs and those checks can be merged to one if() statement.
--- tools/perf/perf.c | 8 +++++--- tools/perf/util/quote.c | 36 ++++++++++++++++++++---------------- tools/perf/util/quote.h | 2 +- 3 files changed, 26 insertions(+), 20 deletions(-)
tools/perf/perf.c +5 -3
@@ -309,9 +309,11 @@ static int handle_alias(int *argcp, const char ***argv)if (*argcp >1 ) {struct strbuf buf; -strbuf_init (&buf,PATH_MAX ); -strbuf_addstr (&buf, alias_string); -sq_quote_argv (&buf, (*argv) +1 ,PATH_MAX ); +if (strbuf_init (&buf,PATH_MAX ) <0 || +strbuf_addstr (&buf, alias_string) <0 || +sq_quote_argv (&buf, (*argv) +1 , +PATH_MAX ) <0 ) +die ("Failed to allocate memory." );free (alias_string); alias_string = buf.buf; }
tools/perf/util/quote.c +20 -16
@@ -17,38 +17,42 @@ static inline int need_bs_quote(char c)return (c =='\'' || c =='!' ); } -static void sq_quote_buf (struct strbuf *dst,const char *src) +static int sq_quote_buf (struct strbuf *dst,const char *src) {char *to_free =NULL ; +int ret;if (dst->buf == src) to_free =strbuf_detach (dst,NULL ); -strbuf_addch (dst,'\'' ); -while (*src) { + ret =strbuf_addch (dst,'\'' ); +while (!ret && *src) {size_t len =strcspn (src,"'!" ); -strbuf_add (dst, src, len); + ret =strbuf_add (dst, src, len); src += len; -while (need_bs_quote (*src)) { -strbuf_addstr (dst,"'\\" ); -strbuf_addch (dst, *src++); -strbuf_addch (dst,'\'' ); - } +while (!ret &&need_bs_quote (*src)) + ret =strbuf_addf (dst,"'\\%c\'" , *src++); } -strbuf_addch (dst,'\'' ); +if (!ret) + ret =strbuf_addch (dst,'\'' );free (to_free); + +return ret; } -void sq_quote_argv (struct strbuf *dst,const char ** argv,size_t maxlen) +int sq_quote_argv (struct strbuf *dst,const char ** argv,size_t maxlen) { -int i; +int i, ret;/* Copy into destination buffer. */ -strbuf_grow (dst,255 ); -for (i =0 ; argv[i]; ++i) { -strbuf_addch (dst,' ' ); -sq_quote_buf (dst, argv[i]); + ret =strbuf_grow (dst,255 ); +for (i =0 ; !ret && argv[i]; ++i) { + ret =strbuf_addch (dst,' ' ); +if (ret) +break ; + ret =sq_quote_buf (dst, argv[i]);if (maxlen && dst->len > maxlen)die ("Too many or long arguments" ); } +return ret; }
tools/perf/util/quote.h +1 -1
@@ -24,6 +24,6 @@* sq_quote() in a real application. */ -void sq_quote_argv (struct strbuf *,const char **argv,size_t maxlen); +int sq_quote_argv (struct strbuf *,const char **argv,size_t maxlen);#endif /* __PERF_QUOTE_H */
PATCH v3 5/8 perf header: Make topology checkers to check return value of strbuf
In reply to Masami HiramatsuMake topology checkers to check the return value of strbuf APIs so that it can detect errors in it.
---
Changes in v3:
- Avoid using STRBUF_INIT and return error directly before initalizing strbuf.
--- tools/perf/util/header.c | 31 ++++++++++++++++++++----------- 1 file changed, 20 insertions(+), 11 deletions(-)
tools/perf/util/header.c +20 -11
@@ -1819,7 +1819,8 @@ static int process_cpu_topology(struct perf_file_section *section, ph->env.nr_sibling_cores = nr; size +=sizeof (u32); -strbuf_init (&sb,128 ); +if (strbuf_init (&sb,128 ) <0 ) + goto free_cpu;for (i =0 ; i < nr; i++) { str =do_read_string (fd, ph);
@@ -1827,7 +1828,8 @@ static int process_cpu_topology(struct perf_file_section *section, goto error;/* include a NULL character at the end */ -strbuf_add (&sb, str,strlen (str) +1 ); +if (strbuf_add (&sb, str,strlen (str) +1 ) <0 ) + goto error; size +=string_size (str);free (str); }
@@ -1849,7 +1851,8 @@ static int process_cpu_topology(struct perf_file_section *section, goto error;/* include a NULL character at the end */ -strbuf_add (&sb, str,strlen (str) +1 ); +if (strbuf_add (&sb, str,strlen (str) +1 ) <0 ) + goto error; size +=string_size (str);free (str); }
@@ -1912,13 +1915,14 @@ static int process_numa_topology(struct perf_file_section *section __maybe_unuse/* nr nodes */ ret =readn (fd, &nr,sizeof (nr));if (ret !=sizeof (nr)) - goto error; +return -1 ;if (ph->needs_swap) nr =bswap_32 (nr); ph->env.nr_numa_nodes = nr; -strbuf_init (&sb,256 ); +if (strbuf_init (&sb,256 ) <0 ) +return -1 ;for (i =0 ; i < nr; i++) {/* node number */
@@ -1940,15 +1944,17 @@ static int process_numa_topology(struct perf_file_section *section __maybe_unuse mem_free =bswap_64 (mem_free); } -strbuf_addf (&sb,"%u:%" PRIu64":%" PRIu64":" , - node, mem_total, mem_free); +if (strbuf_addf (&sb,"%u:%" PRIu64":%" PRIu64":" , + node, mem_total, mem_free) <0 ) + goto error; str =do_read_string (fd, ph);if (!str) goto error;/* include a NULL character at the end */ -strbuf_add (&sb, str,strlen (str) +1 ); +if (strbuf_add (&sb, str,strlen (str) +1 ) <0 ) + goto error;free (str); } ph->env.numa_nodes =strbuf_detach (&sb,NULL );
@@ -1982,7 +1988,8 @@ static int process_pmu_mappings(struct perf_file_section *section __maybe_unused } ph->env.nr_pmu_mappings = pmu_num; -strbuf_init (&sb,128 ); +if (strbuf_init (&sb,128 ) <0 ) +return -1 ;while (pmu_num) {if (readn (fd, &type,sizeof (type)) !=sizeof (type))
@@ -1994,9 +2001,11 @@ static int process_pmu_mappings(struct perf_file_section *section __maybe_unusedif (!name) goto error; -strbuf_addf (&sb,"%u:%s" , type, name); +if (strbuf_addf (&sb,"%u:%s" , type, name) <0 ) + goto error;/* include a NULL character at the end */ -strbuf_add (&sb,"" ,1 ); +if (strbuf_add (&sb,"" ,1 ) <0 ) + goto error;if (!strcmp (name,"msr" )) ph->env.msr_pmu_type = type;
PATCH v3 6/8 perf pmu: Make pmu_formats_string to check return value of strbuf
In reply to Masami HiramatsuMake pmu_formats_string() to check return value of strbuf APIs so that it can detect errors in it.
--- tools/perf/util/pmu.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
tools/perf/util/pmu.c +5 -5
@@ -643,20 +643,20 @@ static int pmu_resolve_param_term(struct parse_events_term *term,static char *pmu_formats_string (struct list_head *formats) {struct perf_pmu_format *format; -char *str; -struct strbuf buf; +char *str =NULL ; +struct strbuf buf =STRBUF_INIT ;unsigned i =0 ;if (!formats)return NULL ; -strbuf_init (&buf,0 );/* sysfs exported terms */ list_for_each_entry (format, formats, list) -strbuf_addf (&buf, i++ ?",%s" :"%s" , - format->name); +if (strbuf_addf (&buf, i++ ?",%s" :"%s" , format->name) <0 ) + goto error; str =strbuf_detach (&buf,NULL ); +error:strbuf_release (&buf);return str;
PATCH v3 7/8 perf help: Do not use ALLOC_GROW in add_cmd_list
In reply to Masami HiramatsuReplace ALLOC_GROW with normal realloc code in add_cmd_list() so that it can handle errors directly.
--- tools/perf/util/help-unknown-cmd.c | 30 ++++++++++++++++++++++-------- 1 file changed, 22 insertions(+), 8 deletions(-)
tools/perf/util/help-unknown-cmd.c +22 -8
@@ -27,16 +27,27 @@ static int levenshtein_compare(const void *p1, const void *p2)return l1 != l2 ? l1 - l2 :strcmp (s1, s2); } -static void add_cmd_list (struct cmdnames *cmds,struct cmdnames *old) +static int add_cmd_list (struct cmdnames *cmds,struct cmdnames *old) { -unsigned int i; - -ALLOC_GROW (cmds->names, cmds->cnt + old->cnt, cmds->alloc); - +unsigned int i, nr = cmds->cnt + old->cnt; +void *tmp; + +if (nr > cmds->alloc) { +/* Choose bigger one to alloc */ +if (alloc_nr (cmds->alloc) < nr) + cmds->alloc = nr; +else + cmds->alloc =alloc_nr (cmds->alloc); + tmp =realloc (cmds->names, cmds->alloc *sizeof (*cmds->names)); +if (!tmp) +return -1 ; + cmds->names = tmp; + }for (i =0 ; i < old->cnt; i++) cmds->names[cmds->cnt++] = old->names[i];zfree (&old->names); old->cnt =0 ; +return 0 ; }const char *help_unknown_cmd (const char *cmd)
@@ -52,8 +63,11 @@ const char *help_unknown_cmd(const char *cmd)load_command_list ("perf-" , &main_cmds, &other_cmds); -add_cmd_list (&main_cmds, &aliases); -add_cmd_list (&main_cmds, &other_cmds); +if (add_cmd_list (&main_cmds, &aliases) <0 || +add_cmd_list (&main_cmds, &other_cmds) <0 ) { +fprintf (stderr,"ERROR: Failed to allocate command list for unknown command.\n" ); + goto end; + }qsort (main_cmds.names, main_cmds.cnt,sizeof (main_cmds.names), cmdname_compare);uniq (&main_cmds);
@@ -99,6 +113,6 @@ const char *help_unknown_cmd(const char *cmd)for (i =0 ; i < n; i++)fprintf (stderr,"\t%s\n" , main_cmds.names[i]->name); } - +end:exit (1 ); }
PATCH v3 8/8 perf tools: Remove xrealloc and ALLOC_GROW
In reply to Masami HiramatsuRemove unused xrealloc() and ALLOC_GROW() from libperf.
--- tools/perf/util/Build | 1 - tools/perf/util/cache.h | 19 ------------------- tools/perf/util/util.h | 6 ------ tools/perf/util/wrapper.c | 29 ----------------------------- 4 files changed, 55 deletions(-) delete mode 100644 tools/perf/util/wrapper.c
tools/perf/util/Build +0 -1
@@ -27,7 +27,6 @@ libperf-y += strlist.o libperf-y += strfilter.o libperf-y += top.o libperf-y += usage.o -libperf-y += wrapper.o libperf-y += dso.o libperf-y += symbol.o libperf-y += symbol_fprintf.o
tools/perf/util/cache.h +0 -19
@@ -40,25 +40,6 @@ int split_cmdline(char *cmdline, const char ***argv);#define alloc_nr (x) (((x)+16)*3/2) -/* -* Realloc the buffer pointed at by variable 'x' so that it can hold -* at least 'nr' entries; the number of entries currently allocated -* is 'alloc', using the standard growing factor alloc_nr() macro. -* -* DO NOT USE any expression with side-effect for 'x' or 'alloc'. -*/ -#define ALLOC_GROW (x, nr, alloc) \ - do { \ - if ((nr) > alloc) { \ - if (alloc_nr(alloc) < (nr)) \ - alloc = (nr); \ - else \ - alloc = alloc_nr(alloc); \ - x = xrealloc((x), alloc * sizeof(*(x))); \ - } \ - } while(0) - -static inline int is_absolute_path (const char *path) {return path[0 ] =='/' ;
tools/perf/util/util.h +0 -6
@@ -160,12 +160,6 @@ static inline char *gitstrchrnul(const char *s, int c) }#endif -/* -* Wrappers: -*/ -void *xrealloc (void *ptr,size_t size) __attribute__((weak)); - -static inline void *zalloc (size_t size) {return calloc (1 , size);
tools/perf/util/wrapper.c deleted +0 -29
@@ -1,29 +0,0 @@ -/* -* Various trivial helper wrappers around standard functions -*/ -#include "cache.h" - -/* -* There's no pack memory to release - but stay close to the Git -* version so wrap this away: -*/ -static inline void release_pack_memory (size_t size__maybe_unused , -int flag__maybe_unused ) -{ -} - -void *xrealloc (void *ptr,size_t size) -{ -void *ret =realloc (ptr, size); -if (!ret && !size) - ret =realloc (ptr,1 ); -if (!ret) { -release_pack_memory (size,-1 ); - ret =realloc (ptr, size); -if (!ret && !size) - ret =realloc (ptr,1 ); -if (!ret) -die ("Out of memory, realloc failed" ); - } -return ret; -}
Arnaldo Carvalho de Melo
maintainerRe: [PATCH perf/core v3 3/8] perf help: Make check_emacsclient_version to check strbuf APIs
In reply to Masami HiramatsuEm Tue, May 10, 2016 at 02:47:17PM +0900, Masami Hiramatsu escreveu:
Quoted 4 lines
Make
check_emacsclient_version()to check the return value of strbuf APIs so that it can handle errors in strbuf.
---
You forgot to add the S-o-B, since you provided it in the previous submission, I'm adding it now.
- Arnaldo
Quoted 54 lines
tools/perf/builtin-help.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-)tools/perf/builtin-help.c +10
-8@@ -61,6 +61,7 @@ static int check_emacsclient_version(void)struct child_process ec_process;const char *argv_ec[] = {"emacsclient" ,"--version" ,NULL };int version; +int ret =-1 ;/* emacsclient prints its version number on stderr */ memset (&ec_process,0 ,sizeof (ec_process));@@ -71,7 +72,10 @@ static int check_emacsclient_version(void)fprintf (stderr,"Failed to start emacsclient.\n" );return -1 ; } -strbuf_read (&buffer, ec_process.err,20 ); +if (strbuf_read (&buffer, ec_process.err,20 ) <0 ) { +fprintf (stderr,"Failed to read emacsclient version\n" ); + goto out; + }close (ec_process.err);/* @@ -82,8 +86,7 @@ static int check_emacsclient_version(void)if (prefixcmp (buffer.buf,"emacsclient" )) {fprintf (stderr,"Failed to parse emacsclient version.\n" ); -strbuf_release (&buffer); -return -1 ; + goto out; } version =atoi (buffer.buf +strlen ("emacsclient" ));@@ -92,12 +95,11 @@ static int check_emacsclient_version(void)fprintf (stderr,"emacsclient version '%d' too old (< 22).\n" , version); -strbuf_release (&buffer); -return -1 ; - } - + }else + ret =0 ; +out:strbuf_release (&buffer); -return 0 ; +return ret; }static void exec_woman_emacs (const char *path,const char *page)
tip-bot for Masami Hiramatsu
merge bot[tip:perf/core] perf tools: Rewrite strbuf not to die()
In reply to Masami Hiramatsuperf tools: Rewrite strbuf not to die()
Commit-ID: 5cea57f30a12443c05e0c5273f35d2fcef00d30a Gitweb: http://git.kernel.org/tip/5cea57f30a12443c05e0c5273f35d2fcef00d30a Author: Masami Hiramatsu <[email protected]> AuthorDate: Tue, 10 May 2016 14:46:58 +0900 Committer: Arnaldo Carvalho de Melo <[email protected]> CommitDate: Tue, 10 May 2016 11:27:58 -0300
perf tools: Rewrite strbuf not to die()
Rewrite strbuf implementation not to use die() nor xrealloc(). Instead of die(), now most of the API returns error code or 0 if succeeded.
--- tools/perf/util/strbuf.c | 93 ++++++++++++++++++++++++++++++++++-------------- tools/perf/util/strbuf.h | 25 +++++++------ 2 files changed, 82 insertions(+), 36 deletions(-)
tools/perf/util/strbuf.c +67 -26
@@ -1,3 +1,4 @@ +#include "debug.h" #include "cache.h" #include <linux/kernel.h>
@@ -17,12 +18,13 @@ int prefixcmp(const char *str, const char *prefix)*/ char strbuf_slopbuf[1 ]; -void strbuf_init (struct strbuf *sb,ssize_t hint) +int strbuf_init (struct strbuf *sb,ssize_t hint) { sb->alloc = sb->len =0 ; sb->buf = strbuf_slopbuf;if (hint) -strbuf_grow (sb, hint); +return strbuf_grow (sb, hint); +return 0 ; }void strbuf_release (struct strbuf *sb)
@@ -42,67 +44,104 @@ char *strbuf_detach(struct strbuf *sb, size_t *sz)return res; } -void strbuf_grow (struct strbuf *sb,size_t extra) +int strbuf_grow (struct strbuf *sb,size_t extra) { -if (sb->len + extra +1 <= sb->len) -die ("you want to use way too much memory" ); -if (!sb->alloc) - sb->buf =NULL ; -ALLOC_GROW (sb->buf, sb->len + extra +1 , sb->alloc); +char *buf; +size_t nr = sb->len + extra +1 ; + +if (nr < sb->alloc) +return 0 ; + +if (nr <= sb->len) +return -E2BIG ; + +if (alloc_nr (sb->alloc) > nr) + nr =alloc_nr (sb->alloc); + +/* +* Note that sb->buf == strbuf_slopbuf if sb->alloc == 0, and it is +* a static variable. Thus we have to avoid passing it to realloc. +*/ + buf =realloc (sb->alloc ? sb->buf :NULL , nr *sizeof (*buf)); +if (!buf) +return -ENOMEM ; + + sb->buf = buf; + sb->alloc = nr; +return 0 ; } -void strbuf_addch (struct strbuf *sb,int c) +int strbuf_addch (struct strbuf *sb,int c) { -strbuf_grow (sb,1 ); +int ret =strbuf_grow (sb,1 ); +if (ret) +return ret; + sb->buf[sb->len++] = c; sb->buf[sb->len] ='\0' ; +return 0 ; } -void strbuf_add (struct strbuf *sb,const void *data,size_t len) +int strbuf_add (struct strbuf *sb,const void *data,size_t len) { -strbuf_grow (sb, len); +int ret =strbuf_grow (sb, len); +if (ret) +return ret; +memcpy (sb->buf + sb->len, data, len); -strbuf_setlen (sb, sb->len + len); +return strbuf_setlen (sb, sb->len + len); } -static void strbuf_addv (struct strbuf *sb,const char *fmt,va_list ap) +static int strbuf_addv (struct strbuf *sb,const char *fmt,va_list ap) { -int len; +int len, ret;va_list ap_saved; -if (!strbuf_avail (sb)) -strbuf_grow (sb,64 ); +if (!strbuf_avail (sb)) { + ret =strbuf_grow (sb,64 ); +if (ret) +return ret; + }va_copy (ap_saved, ap); len =vsnprintf (sb->buf + sb->len, sb->alloc - sb->len, fmt, ap);if (len <0 ) -die ("your vsnprintf is broken" ); +return len;if (len >strbuf_avail (sb)) { -strbuf_grow (sb, len); + ret =strbuf_grow (sb, len); +if (ret) +return ret; len =vsnprintf (sb->buf + sb->len, sb->alloc - sb->len, fmt, ap_saved);va_end (ap_saved);if (len >strbuf_avail (sb)) { -die ("this should not happen, your vsnprintf is broken" ); +pr_debug ("this should not happen, your vsnprintf is broken" ); +return -EINVAL ; } } -strbuf_setlen (sb, sb->len + len); +return strbuf_setlen (sb, sb->len + len); } -void strbuf_addf (struct strbuf *sb,const char *fmt, ...) +int strbuf_addf (struct strbuf *sb,const char *fmt, ...) {va_list ap; +int ret;va_start (ap, fmt); -strbuf_addv (sb, fmt, ap); + ret =strbuf_addv (sb, fmt, ap);va_end (ap); +return ret; }ssize_t strbuf_read (struct strbuf *sb,int fd,ssize_t hint) {size_t oldlen = sb->len;size_t oldalloc = sb->alloc; +int ret; + + ret =strbuf_grow (sb, hint ? hint :8192 ); +if (ret) +return ret; -strbuf_grow (sb, hint ? hint :8192 );for (;;) {ssize_t cnt;
@@ -112,12 +151,14 @@ ssize_t strbuf_read(struct strbuf *sb, int fd, ssize_t hint)strbuf_release (sb); elsestrbuf_setlen (sb ,oldlen ); -return -1 ; +return cnt; }if (!cnt)break ; sb->len += cnt; -strbuf_grow (sb,8192 ); + ret =strbuf_grow (sb,8192 ); +if (ret) +return ret; } sb->buf[sb->len] ='\0' ;
tools/perf/util/strbuf.h +15 -10
@@ -51,7 +51,7 @@ struct strbuf {#define STRBUF_INIT { 0, 0, strbuf_slopbuf }/*----- strbuf life cycle -----*/ -void strbuf_init (struct strbuf *buf,ssize_t hint); +int strbuf_init (struct strbuf *buf,ssize_t hint);void strbuf_release (struct strbuf *buf);char *strbuf_detach (struct strbuf *buf,size_t *);
@@ -60,26 +60,31 @@ static inline ssize_t strbuf_avail(const struct strbuf *sb) {return sb->alloc ? sb->alloc - sb->len -1 :0 ; } -void strbuf_grow (struct strbuf *buf,size_t ); +int strbuf_grow (struct strbuf *buf,size_t ); -static inline void strbuf_setlen (struct strbuf *sb,size_t len) { -if (!sb->alloc) -strbuf_grow (sb,0 ); +static inline int strbuf_setlen (struct strbuf *sb,size_t len) { +int ret; +if (!sb->alloc) { + ret =strbuf_grow (sb,0 ); +if (ret) +return ret; + }assert (len < sb->alloc); sb->len = len; sb->buf[len] ='\0' ; +return 0 ; }/*----- add data in your buffer -----*/ -void strbuf_addch (struct strbuf *sb,int c); +int strbuf_addch (struct strbuf *sb,int c); -void strbuf_add (struct strbuf *buf,const void *,size_t ); -static inline void strbuf_addstr (struct strbuf *sb,const char *s) { -strbuf_add (sb, s,strlen (s)); +int strbuf_add (struct strbuf *buf,const void *,size_t ); +static inline int strbuf_addstr (struct strbuf *sb,const char *s) { +return strbuf_add (sb, s,strlen (s)); } __attribute__((format (printf,2 ,3 ))) -void strbuf_addf (struct strbuf *sb,const char *fmt, ...); +int strbuf_addf (struct strbuf *sb,const char *fmt, ...);/* XXX: if read fails, any partial read is undone */ ssize_t strbuf_read (struct strbuf *,int fd,ssize_t hint);
tip-bot for Masami Hiramatsu
merge bot[tip:perf/core] perf probe: Check the return value of strbuf APIs
In reply to Masami Hiramatsuperf probe: Check the return value of strbuf APIs
Commit-ID: bf4d5f25c90bf2eca8671f2fc4e3d15919cd7f9c Gitweb: http://git.kernel.org/tip/bf4d5f25c90bf2eca8671f2fc4e3d15919cd7f9c Author: Masami Hiramatsu <[email protected]> AuthorDate: Tue, 10 May 2016 14:47:07 +0900 Committer: Arnaldo Carvalho de Melo <[email protected]> CommitDate: Tue, 10 May 2016 11:53:34 -0300
perf probe: Check the return value of strbuf APIs
Check the return value of strbuf APIs in perf-probe related code, so that it can handle errors in strbuf.
--- tools/perf/util/dwarf-aux.c | 52 +++++++-------- tools/perf/util/probe-event.c | 143 +++++++++++++++++++++++++---------------- tools/perf/util/probe-finder.c | 30 +++++---- 3 files changed, 128 insertions(+), 97 deletions(-)
tools/perf/util/dwarf-aux.c +24 -28
@@ -915,8 +915,7 @@ int die_get_typename(Dwarf_Die *vr_die, struct strbuf *buf) tmp ="*" ; else if (tag == DW_TAG_subroutine_type) {/* Function pointer */ -strbuf_add (buf,"(function_type)" ,15 ); -return 0 ; +return strbuf_add (buf,"(function_type)" ,15 ); } else {if (!dwarf_diename (&type))return -ENOENT ;
@@ -927,14 +926,10 @@ int die_get_typename(Dwarf_Die *vr_die, struct strbuf *buf)else if (tag == DW_TAG_enumeration_type) tmp ="enum " ;/* Write a base name */ -strbuf_addf (buf,"%s%s" , tmp,dwarf_diename (&type)); -return 0 ; +return strbuf_addf (buf,"%s%s" , tmp,dwarf_diename (&type)); } ret =die_get_typename (&type, buf); -if (ret ==0 ) -strbuf_addstr (buf, tmp); - -return ret; +return ret ? ret :strbuf_addstr (buf, tmp); }/**
@@ -951,12 +946,10 @@ int die_get_varname(Dwarf_Die *vr_die, struct strbuf *buf) ret =die_get_typename (vr_die, buf);if (ret <0 ) {pr_debug ("Failed to get type, make it unknown.\n" ); -strbuf_add (buf," (unknown_type)" ,14 ); + ret =strbuf_add (buf," (unknown_type)" ,14 ); } -strbuf_addf (buf,"\t%s" ,dwarf_diename (vr_die)); - -return 0 ; +return ret <0 ? ret :strbuf_addf (buf,"\t%s" ,dwarf_diename (vr_die)); }#ifdef HAVE_DWARF_GETLOCATIONS
@@ -999,22 +992,24 @@ static int die_get_var_innermost_scope(Dwarf_Die *sp_die, Dwarf_Die *vr_die, }while ((offset =dwarf_ranges (&scopes[1 ], offset, &base, - &start, &end)) >0 ) { + &start, &end)) >0 ) { start -= entry; end -= entry;if (first) { -strbuf_addf (buf,"@<%s+[%" PRIu64"-%" PRIu64, - name, start, end); + ret =strbuf_addf (buf,"@<%s+[%" PRIu64"-%" PRIu64, + name, start, end); first = false; }else { -strbuf_addf (buf,",%" PRIu64"-%" PRIu64, - start, end); + ret =strbuf_addf (buf,",%" PRIu64"-%" PRIu64, + start, end); } +if (ret <0 ) + goto out; }if (!first) -strbuf_add (buf,"]>" ,2 ); + ret =strbuf_add (buf,"]>" ,2 ); out:free (scopes);
@@ -1054,31 +1049,32 @@ int die_get_var_range(Dwarf_Die *sp_die, Dwarf_Die *vr_die, struct strbuf *buf)if (dwarf_attr (vr_die, DW_AT_location, &attr) ==NULL )return -EINVAL ; -while ((offset =dwarf_getlocations ( - &attr, offset, &base, - &start, &end, &op, &nops)) >0 ) { +while ((offset =dwarf_getlocations (&attr, offset, &base, + &start, &end, &op, &nops)) >0 ) {if (start ==0 ) {/* Single Location Descriptions */ ret =die_get_var_innermost_scope (sp_die, vr_die, buf); -return ret; + goto out; }/* Location Lists */ start -= entry; end -= entry;if (first) { -strbuf_addf (buf,"@<%s+[%" PRIu64"-%" PRIu64, - name, start, end); + ret =strbuf_addf (buf,"@<%s+[%" PRIu64"-%" PRIu64, + name, start, end); first = false; }else { -strbuf_addf (buf,",%" PRIu64"-%" PRIu64, - start, end); + ret =strbuf_addf (buf,",%" PRIu64"-%" PRIu64, + start, end); } +if (ret <0 ) + goto out; }if (!first) -strbuf_add (buf,"]>" ,2 ); - + ret =strbuf_add (buf,"]>" ,2 ); +out: return ret; }#else
tools/perf/util/probe-event.c +86 -57
270 lines not on this page · raw message
tools/perf/util/probe-finder.c +18 -12
@@ -1294,6 +1294,7 @@ static int collect_variables_cb(Dwarf_Die *die_mem, void *data) {struct available_var_finder *af = data;struct variable_list *vl; +struct strbuf buf =STRBUF_INIT ;int tag, ret; vl = &af->vls[af->nvls -1 ];
@@ -1307,25 +1308,26 @@ static int collect_variables_cb(Dwarf_Die *die_mem, void *data)if (ret ==0 || ret == -ERANGE ) {int ret2;bool externs = !af->child; -struct strbuf buf; -strbuf_init (&buf,64 ); +if (strbuf_init (&buf,64 ) <0 ) + goto error;if (probe_conf.show_location_range) { -if (!externs) { -if (ret) -strbuf_add (&buf,"[INV]\t" ,6 ); -else -strbuf_add (&buf,"[VAL]\t" ,6 ); - }else -strbuf_add (&buf,"[EXT]\t" ,6 ); +if (!externs) + ret2 =strbuf_add (&buf, + ret ?"[INV]\t" :"[VAL]\t" ,6 ); +else + ret2 =strbuf_add (&buf,"[EXT]\t" ,6 ); +if (ret2) + goto error; } ret2 =die_get_varname (die_mem, &buf);if (!ret2 && probe_conf.show_location_range && !externs) { -strbuf_addch (&buf,'\t' ); +if (strbuf_addch (&buf,'\t' ) <0 ) + goto error; ret2 =die_get_var_range (&af->pf.sp_die, die_mem, &buf); }
@@ -1334,8 +1336,8 @@ static int collect_variables_cb(Dwarf_Die *die_mem, void *data)if (ret2 ==0 ) {strlist__add (vl->vars,strbuf_detach (&buf,NULL )); - }else -strbuf_release (&buf); + } +strbuf_release (&buf); } }
@@ -1343,6 +1345,10 @@ static int collect_variables_cb(Dwarf_Die *die_mem, void *data)return DIE_FIND_CB_CONTINUE ; else return DIE_FIND_CB_SIBLING; +error: +strbuf_release (&buf); +pr_debug ("Error in strbuf\n" ); +return DIE_FIND_CB_END ; }/* Add a found vars into available variables list */
tip-bot for Masami Hiramatsu
merge bot[tip:perf/core] perf help: Make check_emacsclient_version to check strbuf APIs
In reply to Masami Hiramatsuperf help: Make check_emacsclient_version to check strbuf APIs
Commit-ID: b72ca4039099e953f1ea2dbd58c201b14feb6605 Gitweb: http://git.kernel.org/tip/b72ca4039099e953f1ea2dbd58c201b14feb6605 Author: Masami Hiramatsu <[email protected]> AuthorDate: Tue, 10 May 2016 14:47:17 +0900 Committer: Arnaldo Carvalho de Melo <[email protected]> CommitDate: Tue, 10 May 2016 11:56:14 -0300
perf help: Make check_emacsclient_version to check strbuf APIs
Make check_emacsclient_version() to check the return value of strbuf APIs so that it can handle errors in strbuf.
--- tools/perf/builtin-help.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-)
tools/perf/builtin-help.c +10 -8
@@ -61,6 +61,7 @@ static int check_emacsclient_version(void)struct child_process ec_process;const char *argv_ec[] = {"emacsclient" ,"--version" ,NULL };int version; +int ret =-1 ;/* emacsclient prints its version number on stderr */ memset (&ec_process,0 ,sizeof (ec_process));
@@ -71,7 +72,10 @@ static int check_emacsclient_version(void)fprintf (stderr,"Failed to start emacsclient.\n" );return -1 ; } -strbuf_read (&buffer, ec_process.err,20 ); +if (strbuf_read (&buffer, ec_process.err,20 ) <0 ) { +fprintf (stderr,"Failed to read emacsclient version\n" ); + goto out; + }close (ec_process.err);/*
@@ -82,8 +86,7 @@ static int check_emacsclient_version(void)if (prefixcmp (buffer.buf,"emacsclient" )) {fprintf (stderr,"Failed to parse emacsclient version.\n" ); -strbuf_release (&buffer); -return -1 ; + goto out; } version =atoi (buffer.buf +strlen ("emacsclient" ));
@@ -92,12 +95,11 @@ static int check_emacsclient_version(void)fprintf (stderr,"emacsclient version '%d' too old (< 22).\n" , version); -strbuf_release (&buffer); -return -1 ; - } - + }else + ret =0 ; +out:strbuf_release (&buffer); -return 0 ; +return ret; }static void exec_woman_emacs (const char *path,const char *page)
tip-bot for Masami Hiramatsu
merge bot[tip:perf/core] perf tools: Make alias handler to check return value of strbuf
In reply to Masami Hiramatsuperf tools: Make alias handler to check return value of strbuf
Commit-ID: 70a6898fdc11272249622f77b034f47f1e9adb35 Gitweb: http://git.kernel.org/tip/70a6898fdc11272249622f77b034f47f1e9adb35 Author: Masami Hiramatsu <[email protected]> AuthorDate: Tue, 10 May 2016 14:47:26 +0900 Committer: Arnaldo Carvalho de Melo <[email protected]> CommitDate: Tue, 10 May 2016 11:56:52 -0300
perf tools: Make alias handler to check return value of strbuf
Make alias handler and sq_quote_argv to check the return value of strbuf APIs.
In sq_quote_argv() calls die(), but this fix handles strbuf failure as a special case and returns to caller, since the caller - handle_alias() also has to check the return value of other strbuf APIs and those checks can be merged to one if() statement.
--- tools/perf/perf.c | 8 +++++--- tools/perf/util/quote.c | 36 ++++++++++++++++++++---------------- tools/perf/util/quote.h | 2 +- 3 files changed, 26 insertions(+), 20 deletions(-)
tools/perf/perf.c +5 -3
@@ -309,9 +309,11 @@ static int handle_alias(int *argcp, const char ***argv)if (*argcp >1 ) {struct strbuf buf; -strbuf_init (&buf,PATH_MAX ); -strbuf_addstr (&buf, alias_string); -sq_quote_argv (&buf, (*argv) +1 ,PATH_MAX ); +if (strbuf_init (&buf,PATH_MAX ) <0 || +strbuf_addstr (&buf, alias_string) <0 || +sq_quote_argv (&buf, (*argv) +1 , +PATH_MAX ) <0 ) +die ("Failed to allocate memory." );free (alias_string); alias_string = buf.buf; }
tools/perf/util/quote.c +20 -16
@@ -17,38 +17,42 @@ static inline int need_bs_quote(char c)return (c =='\'' || c =='!' ); } -static void sq_quote_buf (struct strbuf *dst,const char *src) +static int sq_quote_buf (struct strbuf *dst,const char *src) {char *to_free =NULL ; +int ret;if (dst->buf == src) to_free =strbuf_detach (dst,NULL ); -strbuf_addch (dst,'\'' ); -while (*src) { + ret =strbuf_addch (dst,'\'' ); +while (!ret && *src) {size_t len =strcspn (src,"'!" ); -strbuf_add (dst, src, len); + ret =strbuf_add (dst, src, len); src += len; -while (need_bs_quote (*src)) { -strbuf_addstr (dst,"'\\" ); -strbuf_addch (dst, *src++); -strbuf_addch (dst,'\'' ); - } +while (!ret &&need_bs_quote (*src)) + ret =strbuf_addf (dst,"'\\%c\'" , *src++); } -strbuf_addch (dst,'\'' ); +if (!ret) + ret =strbuf_addch (dst,'\'' );free (to_free); + +return ret; } -void sq_quote_argv (struct strbuf *dst,const char ** argv,size_t maxlen) +int sq_quote_argv (struct strbuf *dst,const char ** argv,size_t maxlen) { -int i; +int i, ret;/* Copy into destination buffer. */ -strbuf_grow (dst,255 ); -for (i =0 ; argv[i]; ++i) { -strbuf_addch (dst,' ' ); -sq_quote_buf (dst, argv[i]); + ret =strbuf_grow (dst,255 ); +for (i =0 ; !ret && argv[i]; ++i) { + ret =strbuf_addch (dst,' ' ); +if (ret) +break ; + ret =sq_quote_buf (dst, argv[i]);if (maxlen && dst->len > maxlen)die ("Too many or long arguments" ); } +return ret; }
tools/perf/util/quote.h +1 -1
@@ -24,6 +24,6 @@* sq_quote() in a real application. */ -void sq_quote_argv (struct strbuf *,const char **argv,size_t maxlen); +int sq_quote_argv (struct strbuf *,const char **argv,size_t maxlen);#endif /* __PERF_QUOTE_H */
tip-bot for Masami Hiramatsu
merge bot[tip:perf/core] perf header: Make topology checkers to check return value of strbuf
In reply to Masami Hiramatsuperf header: Make topology checkers to check return value of strbuf
Commit-ID: 642aadaa320bf9466fd12e3c0903977410bcb731 Gitweb: http://git.kernel.org/tip/642aadaa320bf9466fd12e3c0903977410bcb731 Author: Masami Hiramatsu <[email protected]> AuthorDate: Tue, 10 May 2016 14:47:35 +0900 Committer: Arnaldo Carvalho de Melo <[email protected]> CommitDate: Tue, 10 May 2016 11:57:22 -0300
perf header: Make topology checkers to check return value of strbuf
Make topology checkers to check the return value of strbuf APIs so that it can detect errors in it.
--- tools/perf/util/header.c | 31 ++++++++++++++++++++----------- 1 file changed, 20 insertions(+), 11 deletions(-)
tools/perf/util/header.c +20 -11
@@ -1819,7 +1819,8 @@ static int process_cpu_topology(struct perf_file_section *section, ph->env.nr_sibling_cores = nr; size +=sizeof (u32); -strbuf_init (&sb,128 ); +if (strbuf_init (&sb,128 ) <0 ) + goto free_cpu;for (i =0 ; i < nr; i++) { str =do_read_string (fd, ph);
@@ -1827,7 +1828,8 @@ static int process_cpu_topology(struct perf_file_section *section, goto error;/* include a NULL character at the end */ -strbuf_add (&sb, str,strlen (str) +1 ); +if (strbuf_add (&sb, str,strlen (str) +1 ) <0 ) + goto error; size +=string_size (str);free (str); }
@@ -1849,7 +1851,8 @@ static int process_cpu_topology(struct perf_file_section *section, goto error;/* include a NULL character at the end */ -strbuf_add (&sb, str,strlen (str) +1 ); +if (strbuf_add (&sb, str,strlen (str) +1 ) <0 ) + goto error; size +=string_size (str);free (str); }
@@ -1912,13 +1915,14 @@ static int process_numa_topology(struct perf_file_section *section __maybe_unuse/* nr nodes */ ret =readn (fd, &nr,sizeof (nr));if (ret !=sizeof (nr)) - goto error; +return -1 ;if (ph->needs_swap) nr =bswap_32 (nr); ph->env.nr_numa_nodes = nr; -strbuf_init (&sb,256 ); +if (strbuf_init (&sb,256 ) <0 ) +return -1 ;for (i =0 ; i < nr; i++) {/* node number */
@@ -1940,15 +1944,17 @@ static int process_numa_topology(struct perf_file_section *section __maybe_unuse mem_free =bswap_64 (mem_free); } -strbuf_addf (&sb,"%u:%" PRIu64":%" PRIu64":" , - node, mem_total, mem_free); +if (strbuf_addf (&sb,"%u:%" PRIu64":%" PRIu64":" , + node, mem_total, mem_free) <0 ) + goto error; str =do_read_string (fd, ph);if (!str) goto error;/* include a NULL character at the end */ -strbuf_add (&sb, str,strlen (str) +1 ); +if (strbuf_add (&sb, str,strlen (str) +1 ) <0 ) + goto error;free (str); } ph->env.numa_nodes =strbuf_detach (&sb,NULL );
@@ -1982,7 +1988,8 @@ static int process_pmu_mappings(struct perf_file_section *section __maybe_unused } ph->env.nr_pmu_mappings = pmu_num; -strbuf_init (&sb,128 ); +if (strbuf_init (&sb,128 ) <0 ) +return -1 ;while (pmu_num) {if (readn (fd, &type,sizeof (type)) !=sizeof (type))
@@ -1994,9 +2001,11 @@ static int process_pmu_mappings(struct perf_file_section *section __maybe_unusedif (!name) goto error; -strbuf_addf (&sb,"%u:%s" , type, name); +if (strbuf_addf (&sb,"%u:%s" , type, name) <0 ) + goto error;/* include a NULL character at the end */ -strbuf_add (&sb,"" ,1 ); +if (strbuf_add (&sb,"" ,1 ) <0 ) + goto error;if (!strcmp (name,"msr" )) ph->env.msr_pmu_type = type;
tip-bot for Masami Hiramatsu
merge bot[tip:perf/core] perf pmu: Make pmu_formats_string to check return value of strbuf
In reply to Masami Hiramatsuperf pmu: Make pmu_formats_string to check return value of strbuf
Commit-ID: 11db4e29bb50442ecef2173f325b7be4e7790025 Gitweb: http://git.kernel.org/tip/11db4e29bb50442ecef2173f325b7be4e7790025 Author: Masami Hiramatsu <[email protected]> AuthorDate: Tue, 10 May 2016 14:47:44 +0900 Committer: Arnaldo Carvalho de Melo <[email protected]> CommitDate: Tue, 10 May 2016 11:57:52 -0300
perf pmu: Make pmu_formats_string to check return value of strbuf
Make pmu_formats_string() to check return value of strbuf APIs so that it can detect errors in it.
--- tools/perf/util/pmu.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
tools/perf/util/pmu.c +5 -5
@@ -643,20 +643,20 @@ static int pmu_resolve_param_term(struct parse_events_term *term,static char *pmu_formats_string (struct list_head *formats) {struct perf_pmu_format *format; -char *str; -struct strbuf buf; +char *str =NULL ; +struct strbuf buf =STRBUF_INIT ;unsigned i =0 ;if (!formats)return NULL ; -strbuf_init (&buf,0 );/* sysfs exported terms */ list_for_each_entry (format, formats, list) -strbuf_addf (&buf, i++ ?",%s" :"%s" , - format->name); +if (strbuf_addf (&buf, i++ ?",%s" :"%s" , format->name) <0 ) + goto error; str =strbuf_detach (&buf,NULL ); +error:strbuf_release (&buf);return str;
tip-bot for Masami Hiramatsu
merge bot[tip:perf/core] perf help: Do not use ALLOC_GROW in add_cmd_list
In reply to Masami Hiramatsuperf help: Do not use ALLOC_GROW in add_cmd_list
Commit-ID: 682f4f035e0fcffce511fe77a02a0f19f0996d70 Gitweb: http://git.kernel.org/tip/682f4f035e0fcffce511fe77a02a0f19f0996d70 Author: Masami Hiramatsu <[email protected]> AuthorDate: Tue, 10 May 2016 14:47:53 +0900 Committer: Arnaldo Carvalho de Melo <[email protected]> CommitDate: Tue, 10 May 2016 11:58:09 -0300
perf help: Do not use ALLOC_GROW in add_cmd_list
Replace ALLOC_GROW with normal realloc code in add_cmd_list() so that it can handle errors directly.
--- tools/perf/util/help-unknown-cmd.c | 30 ++++++++++++++++++++++-------- 1 file changed, 22 insertions(+), 8 deletions(-)
tools/perf/util/help-unknown-cmd.c +22 -8
@@ -27,16 +27,27 @@ static int levenshtein_compare(const void *p1, const void *p2)return l1 != l2 ? l1 - l2 :strcmp (s1, s2); } -static void add_cmd_list (struct cmdnames *cmds,struct cmdnames *old) +static int add_cmd_list (struct cmdnames *cmds,struct cmdnames *old) { -unsigned int i; - -ALLOC_GROW (cmds->names, cmds->cnt + old->cnt, cmds->alloc); - +unsigned int i, nr = cmds->cnt + old->cnt; +void *tmp; + +if (nr > cmds->alloc) { +/* Choose bigger one to alloc */ +if (alloc_nr (cmds->alloc) < nr) + cmds->alloc = nr; +else + cmds->alloc =alloc_nr (cmds->alloc); + tmp =realloc (cmds->names, cmds->alloc *sizeof (*cmds->names)); +if (!tmp) +return -1 ; + cmds->names = tmp; + }for (i =0 ; i < old->cnt; i++) cmds->names[cmds->cnt++] = old->names[i];zfree (&old->names); old->cnt =0 ; +return 0 ; }const char *help_unknown_cmd (const char *cmd)
@@ -52,8 +63,11 @@ const char *help_unknown_cmd(const char *cmd)load_command_list ("perf-" , &main_cmds, &other_cmds); -add_cmd_list (&main_cmds, &aliases); -add_cmd_list (&main_cmds, &other_cmds); +if (add_cmd_list (&main_cmds, &aliases) <0 || +add_cmd_list (&main_cmds, &other_cmds) <0 ) { +fprintf (stderr,"ERROR: Failed to allocate command list for unknown command.\n" ); + goto end; + }qsort (main_cmds.names, main_cmds.cnt,sizeof (main_cmds.names), cmdname_compare);uniq (&main_cmds);
@@ -99,6 +113,6 @@ const char *help_unknown_cmd(const char *cmd)for (i =0 ; i < n; i++)fprintf (stderr,"\t%s\n" , main_cmds.names[i]->name); } - +end:exit (1 ); }
tip-bot for Masami Hiramatsu
merge bot[tip:perf/core] perf tools: Remove xrealloc and ALLOC_GROW
In reply to Masami Hiramatsuperf tools: Remove xrealloc and ALLOC_GROW
Commit-ID: 452e84012595d681f254a3a0d733fb0b18ffaf42 Gitweb: http://git.kernel.org/tip/452e84012595d681f254a3a0d733fb0b18ffaf42 Author: Masami Hiramatsu <[email protected]> AuthorDate: Tue, 10 May 2016 14:48:01 +0900 Committer: Arnaldo Carvalho de Melo <[email protected]> CommitDate: Tue, 10 May 2016 11:58:27 -0300
perf tools: Remove xrealloc and ALLOC_GROW
Remove unused xrealloc() and ALLOC_GROW() from libperf.
--- tools/perf/util/Build | 1 - tools/perf/util/cache.h | 19 ------------------- tools/perf/util/util.h | 6 ------ tools/perf/util/wrapper.c | 29 ----------------------------- 4 files changed, 55 deletions(-)
tools/perf/util/Build +0 -1
@@ -27,7 +27,6 @@ libperf-y += strlist.o libperf-y += strfilter.o libperf-y += top.o libperf-y += usage.o -libperf-y += wrapper.o libperf-y += dso.o libperf-y += symbol.o libperf-y += symbol_fprintf.o
tools/perf/util/cache.h +0 -19
@@ -40,25 +40,6 @@ int split_cmdline(char *cmdline, const char ***argv);#define alloc_nr (x) (((x)+16)*3/2) -/* -* Realloc the buffer pointed at by variable 'x' so that it can hold -* at least 'nr' entries; the number of entries currently allocated -* is 'alloc', using the standard growing factor alloc_nr() macro. -* -* DO NOT USE any expression with side-effect for 'x' or 'alloc'. -*/ -#define ALLOC_GROW (x, nr, alloc) \ - do { \ - if ((nr) > alloc) { \ - if (alloc_nr(alloc) < (nr)) \ - alloc = (nr); \ - else \ - alloc = alloc_nr(alloc); \ - x = xrealloc((x), alloc * sizeof(*(x))); \ - } \ - } while(0) - -static inline int is_absolute_path (const char *path) {return path[0 ] =='/' ;
tools/perf/util/util.h +0 -6
@@ -160,12 +160,6 @@ static inline char *gitstrchrnul(const char *s, int c) }#endif -/* -* Wrappers: -*/ -void *xrealloc (void *ptr,size_t size) __attribute__((weak)); - -static inline void *zalloc (size_t size) {return calloc (1 , size);
tools/perf/util/wrapper.c deleted +0 -29
@@ -1,29 +0,0 @@ -/* -* Various trivial helper wrappers around standard functions -*/ -#include "cache.h" - -/* -* There's no pack memory to release - but stay close to the Git -* version so wrap this away: -*/ -static inline void release_pack_memory (size_t size__maybe_unused , -int flag__maybe_unused ) -{ -} - -void *xrealloc (void *ptr,size_t size) -{ -void *ret =realloc (ptr, size); -if (!ret && !size) - ret =realloc (ptr,1 ); -if (!ret) { -release_pack_memory (size,-1 ); - ret =realloc (ptr, size); -if (!ret && !size) - ret =realloc (ptr,1 ); -if (!ret) -die ("Out of memory, realloc failed" ); - } -return ret; -}