Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1391231 > unrolled thread

[PATCH perf/core v2 0/8] perf tools: Update strbuf to remove xrealloc

Back to article view | Back to linux.kernel

Contents

  [PATCH perf/core v2 0/8] perf tools: Update strbuf to remove xrealloc Masami Hiramatsu <mhiramat@kernel.org> - 2016-04-29 17:10 +0200
    [PATCH perf/core v2 2/8] perf probe: Check the return value of strbuf APIs Masami Hiramatsu <mhiramat@kernel.org> - 2016-04-29 17:20 +0200
      Re: [PATCH perf/core v2 2/8] perf probe: Check the return value of  strbuf APIs Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-05-06 01:50 +0200
        Re: [PATCH perf/core v2 2/8] perf probe: Check the return value of  strbuf APIs Masami Hiramatsu <mhiramat@kernel.org> - 2016-05-10 04:40 +0200
    [PATCH perf/core v2 5/8] perf header: Make topology checkers to check return value of strbuf Masami Hiramatsu <mhiramat@kernel.org> - 2016-04-29 17:20 +0200
      Re: [PATCH perf/core v2 5/8] perf header: Make topology checkers to  check return value of strbuf Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-05-06 02:00 +0200
        Re: [PATCH perf/core v2 5/8] perf header: Make topology checkers to  check return value of strbuf Masami Hiramatsu <mhiramat@kernel.org> - 2016-05-10 05:00 +0200
    [PATCH perf/core v2 8/8] perf tools: Remove xrealloc and ALLOC_GROW Masami Hiramatsu <mhiramat@kernel.org> - 2016-04-29 17:20 +0200
      Re: [PATCH perf/core v2 8/8] perf tools: Remove xrealloc and  ALLOC_GROW Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-05-06 02:00 +0200
    [PATCH perf/core v2 3/8] perf help: Make check_emacsclient_version to check strbuf APIs Masami Hiramatsu <mhiramat@kernel.org> - 2016-04-29 17:20 +0200
      Re: [PATCH perf/core v2 3/8] perf help: Make  check_emacsclient_version to check strbuf APIs Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-05-06 02:00 +0200
    [PATCH perf/core v2 4/8] perf: Make alias handler to check return value of strbuf Masami Hiramatsu <mhiramat@kernel.org> - 2016-04-29 17:20 +0200
      Re: [PATCH perf/core v2 4/8] perf: Make alias handler to check  return value of strbuf Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-05-06 02:00 +0200
    [PATCH perf/core v2 6/8] perf pmu: Make pmu_formats_string to check return value of strbuf Masami Hiramatsu <mhiramat@kernel.org> - 2016-04-29 17:20 +0200
      Re: [PATCH perf/core v2 6/8] perf pmu: Make pmu_formats_string to  check return value of strbuf Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-05-06 02:00 +0200
    [PATCH perf/core v2 7/8] perf help: Do not use ALLOC_GROW in add_cmd_list Masami Hiramatsu <mhiramat@kernel.org> - 2016-04-29 17:20 +0200
      Re: [PATCH perf/core v2 7/8] perf help: Do not use ALLOC_GROW in  add_cmd_list Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-05-06 02:00 +0200
    Re: [PATCH perf/core v2 0/8] perf tools: Update strbuf to remove  xrealloc Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-04-29 17:20 +0200
    [PATCH perf/core v2 1/8] perf: Rewrite strbuf not to die Masami Hiramatsu <mhiramat@kernel.org> - 2016-04-29 17:20 +0200
      Re: [PATCH perf/core v2 1/8] perf: Rewrite strbuf not to die Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-05-06 01:30 +0200
        Re: [PATCH perf/core v2 1/8] perf: Rewrite strbuf not to die Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-05-06 01:50 +0200
        Re: [PATCH perf/core v2 1/8] perf: Rewrite strbuf not to die Masami Hiramatsu <mhiramat@kernel.org> - 2016-05-10 03:10 +0200

Page 2 of 2 — ← Prev page 1 [2]

#1395494 — Re: [PATCH perf/core v2 1/8] perf: Rewrite strbuf not to die

Em Thu, May 05, 2016 at 08:25:38PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Sat, Apr 30, 2016 at 12:09:50AM +0900, Masami Hiramatsu escreveu:
> > @@ -112,12 +151,14 @@ ssize_t strbuf_read(struct strbuf *sb, int fd, ssize_t hint)
> >  				strbuf_release(sb);
> >  			else
> >  				strbuf_setlen(sb, oldlen);
> > -			return -1;
> > +			return cnt;
> 
> This is unrelated, no?
> 
> I.e. this _was_ already returning a failure code, but then you are
> propagating the read() return, which may even be a good idea, haven't
> thought about that, but is unrelated to what this patch is doing, please
> put it in a separate patch if you think it is a good idea.
> 
> All the rest seems ok, going over the other patches now.

Just a bit annoying, but this is done when checking if cnt < 0, which
can only be -1 as per read's man page, so its ok, nevermind...

- Arnaldo

[toc] | [prev] | [next] | [standalone]

#1397608 — Re: [PATCH perf/core v2 1/8] perf: Rewrite strbuf not to die

On Thu, 5 May 2016 20:25:38 -0300
Arnaldo Carvalho de Melo <acme@kernel.org> wrote:

> Em Sat, Apr 30, 2016 at 12:09:50AM +0900, Masami Hiramatsu escreveu:
> > Rewrite strbuf implementation not to use die() nor xrealloc().
> > Instead of die, now most of the API returns error code or 0 if
> > succeeded.
> > 
> > Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> > ---
> >  tools/perf/util/strbuf.c |   93 +++++++++++++++++++++++++++++++++-------------
> >  tools/perf/util/strbuf.h |   25 +++++++-----
> >  2 files changed, 82 insertions(+), 36 deletions(-)
> > 
> > diff --git a/tools/perf/util/strbuf.c b/tools/perf/util/strbuf.c
> > index 8fb7329..a98bb60 100644
> > --- a/tools/perf/util/strbuf.c
> > +++ b/tools/perf/util/strbuf.c
> > @@ -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);
> > +
> > +	buf = malloc(nr * sizeof(*buf));
> > +	if (!buf)
> > +		return -ENOMEM;
> > +
> > +	if (sb->alloc) {
> > +		memcpy(buf, sb->buf, sb->alloc);
> > +		free(sb->buf);
> > +	}
> 
> Why not use realloc? I.e. as the old code did, the problem was not that,
> was just the panic when the realloc operation fails, that you are
> removing here.

Oops, right. it seems I misunderstood the return value of realloc.
> 
> I.e. the above would be:
> 
> 	buf = realloc(sb->buf, nr * sizeof(*buf));
> 	if (!buf)
> 		return -ENOMEM;
> 
> > +	sb->buf = buf;
> > +	sb->alloc = nr;
> > +	return 0;
> 
> I.e. no need to do the memcpy() nor the free(), no?

Yes, agreed.
I'll update to do so.

Thanks!




-- 
Masami Hiramatsu <mhiramat@kernel.org>

[toc] | [prev] | [standalone]

Page 2 of 2 — ← Prev page 1 [2]

Back to top | Article view | linux.kernel

csiph-web