* [PATCH -next] tools build: Check return value of fwrite_unlocked in jvmti_agent.c @ 2020-07-24 10:07 Wang ShaoBo 2020-07-28 12:10 ` Arnaldo Carvalho de Melo 2020-07-29 23:47 ` Ian Rogers 0 siblings, 2 replies; 6+ messages in thread From: Wang ShaoBo @ 2020-07-24 10:07 UTC (permalink / raw) Cc: cj.chengjian, huawei.libin, jolsa, acme, linux-kernel, linux-perf-users Function jvmti_write_code called by compiled_method_load_cb may return error in using fwrite_unlocked, this failure should be captured and warned. Signed-off-by: Wang ShaoBo <bobo.shaobowang@huawei.com> --- tools/perf/jvmti/jvmti_agent.c | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/tools/perf/jvmti/jvmti_agent.c b/tools/perf/jvmti/jvmti_agent.c index 88108598d6e9..a1fe6aa16b6d 100644 --- a/tools/perf/jvmti/jvmti_agent.c +++ b/tools/perf/jvmti/jvmti_agent.c @@ -363,7 +363,7 @@ jvmti_write_code(void *agent, char const *sym, struct jr_code_load rec; size_t sym_len; FILE *fp = agent; - int ret = -1; + int ret; /* don't care about 0 length function, no samples */ if (size == 0) @@ -401,16 +401,23 @@ jvmti_write_code(void *agent, char const *sym, rec.code_index = code_generation++; ret = fwrite_unlocked(&rec, sizeof(rec), 1, fp); - fwrite_unlocked(sym, sym_len, 1, fp); + if (ret) + goto error; + ret = fwrite_unlocked(sym, sym_len, 1, fp); + if (ret) + goto error; - if (code) - fwrite_unlocked(code, size, 1, fp); + if (code) { + ret = fwrite_unlocked(code, size, 1, fp); + if (ret) + goto error; + } funlockfile(fp); - - ret = 0; - - return ret; + return 0; +error: + funlockfile(fp); + return -1; } int -- 2.17.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH -next] tools build: Check return value of fwrite_unlocked in jvmti_agent.c 2020-07-24 10:07 [PATCH -next] tools build: Check return value of fwrite_unlocked in jvmti_agent.c Wang ShaoBo @ 2020-07-28 12:10 ` Arnaldo Carvalho de Melo 2020-07-29 23:47 ` Ian Rogers 1 sibling, 0 replies; 6+ messages in thread From: Arnaldo Carvalho de Melo @ 2020-07-28 12:10 UTC (permalink / raw) To: Wang ShaoBo Cc: cj.chengjian, huawei.libin, jolsa, linux-kernel, linux-perf-users Em Fri, Jul 24, 2020 at 06:07:06PM +0800, Wang ShaoBo escreveu: > Function jvmti_write_code called by compiled_method_load_cb may return > error in using fwrite_unlocked, this failure should be captured and > warned. Thanks, applied. - Arnaldo > Signed-off-by: Wang ShaoBo <bobo.shaobowang@huawei.com> > --- > tools/perf/jvmti/jvmti_agent.c | 23 +++++++++++++++-------- > 1 file changed, 15 insertions(+), 8 deletions(-) > > diff --git a/tools/perf/jvmti/jvmti_agent.c b/tools/perf/jvmti/jvmti_agent.c > index 88108598d6e9..a1fe6aa16b6d 100644 > --- a/tools/perf/jvmti/jvmti_agent.c > +++ b/tools/perf/jvmti/jvmti_agent.c > @@ -363,7 +363,7 @@ jvmti_write_code(void *agent, char const *sym, > struct jr_code_load rec; > size_t sym_len; > FILE *fp = agent; > - int ret = -1; > + int ret; > > /* don't care about 0 length function, no samples */ > if (size == 0) > @@ -401,16 +401,23 @@ jvmti_write_code(void *agent, char const *sym, > rec.code_index = code_generation++; > > ret = fwrite_unlocked(&rec, sizeof(rec), 1, fp); > - fwrite_unlocked(sym, sym_len, 1, fp); > + if (ret) > + goto error; > + ret = fwrite_unlocked(sym, sym_len, 1, fp); > + if (ret) > + goto error; > > - if (code) > - fwrite_unlocked(code, size, 1, fp); > + if (code) { > + ret = fwrite_unlocked(code, size, 1, fp); > + if (ret) > + goto error; > + } > > funlockfile(fp); > - > - ret = 0; > - > - return ret; > + return 0; > +error: > + funlockfile(fp); > + return -1; > } > > int > -- > 2.17.1 > -- - Arnaldo ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH -next] tools build: Check return value of fwrite_unlocked in jvmti_agent.c 2020-07-24 10:07 [PATCH -next] tools build: Check return value of fwrite_unlocked in jvmti_agent.c Wang ShaoBo 2020-07-28 12:10 ` Arnaldo Carvalho de Melo @ 2020-07-29 23:47 ` Ian Rogers 2020-07-30 1:12 ` Arnaldo Carvalho de Melo 2020-07-30 10:03 ` Arnaldo Carvalho de Melo 1 sibling, 2 replies; 6+ messages in thread From: Ian Rogers @ 2020-07-29 23:47 UTC (permalink / raw) To: Wang ShaoBo Cc: cj.chengjian, Li Bin, Jiri Olsa, Arnaldo Carvalho de Melo, LKML, linux-perf-users On Fri, Jul 24, 2020 at 3:07 AM Wang ShaoBo <bobo.shaobowang@huawei.com> wrote: > > Function jvmti_write_code called by compiled_method_load_cb may return > error in using fwrite_unlocked, this failure should be captured and > warned. > > Signed-off-by: Wang ShaoBo <bobo.shaobowang@huawei.com> > --- > tools/perf/jvmti/jvmti_agent.c | 23 +++++++++++++++-------- > 1 file changed, 15 insertions(+), 8 deletions(-) > > diff --git a/tools/perf/jvmti/jvmti_agent.c b/tools/perf/jvmti/jvmti_agent.c > index 88108598d6e9..a1fe6aa16b6d 100644 > --- a/tools/perf/jvmti/jvmti_agent.c > +++ b/tools/perf/jvmti/jvmti_agent.c > @@ -363,7 +363,7 @@ jvmti_write_code(void *agent, char const *sym, > struct jr_code_load rec; > size_t sym_len; > FILE *fp = agent; > - int ret = -1; > + int ret; > > /* don't care about 0 length function, no samples */ > if (size == 0) > @@ -401,16 +401,23 @@ jvmti_write_code(void *agent, char const *sym, > rec.code_index = code_generation++; > > ret = fwrite_unlocked(&rec, sizeof(rec), 1, fp); > - fwrite_unlocked(sym, sym_len, 1, fp); > + if (ret) > + goto error; Sorry, it seems I lost a reply to this. Won't ret here be the number of items written and not an error code? Consequently all writes will immediately goto error? Thanks, Ian > + ret = fwrite_unlocked(sym, sym_len, 1, fp); > + if (ret) > + goto error; > > - if (code) > - fwrite_unlocked(code, size, 1, fp); > + if (code) { > + ret = fwrite_unlocked(code, size, 1, fp); > + if (ret) > + goto error; > + } > > funlockfile(fp); > - > - ret = 0; > - > - return ret; > + return 0; > +error: > + funlockfile(fp); > + return -1; > } > > int > -- > 2.17.1 > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH -next] tools build: Check return value of fwrite_unlocked in jvmti_agent.c 2020-07-29 23:47 ` Ian Rogers @ 2020-07-30 1:12 ` Arnaldo Carvalho de Melo 2020-07-30 10:03 ` Arnaldo Carvalho de Melo 1 sibling, 0 replies; 6+ messages in thread From: Arnaldo Carvalho de Melo @ 2020-07-30 1:12 UTC (permalink / raw) To: Ian Rogers, Wang ShaoBo Cc: cj.chengjian, Li Bin, Jiri Olsa, Arnaldo Carvalho de Melo, LKML, linux-perf-users On July 29, 2020 8:47:36 PM GMT-03:00, Ian Rogers <irogers@google.com> wrote: >On Fri, Jul 24, 2020 at 3:07 AM Wang ShaoBo ><bobo.shaobowang@huawei.com> wrote: >> >> Function jvmti_write_code called by compiled_method_load_cb may >return >> error in using fwrite_unlocked, this failure should be captured and >> warned. >> >> Signed-off-by: Wang ShaoBo <bobo.shaobowang@huawei.com> >> --- >> tools/perf/jvmti/jvmti_agent.c | 23 +++++++++++++++-------- >> 1 file changed, 15 insertions(+), 8 deletions(-) >> >> diff --git a/tools/perf/jvmti/jvmti_agent.c >b/tools/perf/jvmti/jvmti_agent.c >> index 88108598d6e9..a1fe6aa16b6d 100644 >> --- a/tools/perf/jvmti/jvmti_agent.c >> +++ b/tools/perf/jvmti/jvmti_agent.c >> @@ -363,7 +363,7 @@ jvmti_write_code(void *agent, char const *sym, >> struct jr_code_load rec; >> size_t sym_len; >> FILE *fp = agent; >> - int ret = -1; >> + int ret; >> >> /* don't care about 0 length function, no samples */ >> if (size == 0) >> @@ -401,16 +401,23 @@ jvmti_write_code(void *agent, char const *sym, >> rec.code_index = code_generation++; >> >> ret = fwrite_unlocked(&rec, sizeof(rec), 1, fp); >> - fwrite_unlocked(sym, sym_len, 1, fp); >> + if (ret) >> + goto error; > >Sorry, it seems I lost a reply to this. Won't ret here be the number >of items written and not an error code? Consequently all writes will >immediately goto error? Good thing this is in tmp.perf/core, you're right, the test has to be (ret < 0)... I'll fix, thanks! Also we need some 'perf test' for this :-/ - Arnaldo > >Thanks, >Ian > >> + ret = fwrite_unlocked(sym, sym_len, 1, fp); >> + if (ret) >> + goto error; >> >> - if (code) >> - fwrite_unlocked(code, size, 1, fp); >> + if (code) { >> + ret = fwrite_unlocked(code, size, 1, fp); >> + if (ret) >> + goto error; >> + } >> >> funlockfile(fp); >> - >> - ret = 0; >> - >> - return ret; >> + return 0; >> +error: >> + funlockfile(fp); >> + return -1; >> } >> >> int >> -- >> 2.17.1 >> -- Sent from my Android device with K-9 Mail. Please excuse my brevity. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH -next] tools build: Check return value of fwrite_unlocked in jvmti_agent.c 2020-07-29 23:47 ` Ian Rogers 2020-07-30 1:12 ` Arnaldo Carvalho de Melo @ 2020-07-30 10:03 ` Arnaldo Carvalho de Melo 2020-07-31 11:58 ` Wangshaobo (bobo) 1 sibling, 1 reply; 6+ messages in thread From: Arnaldo Carvalho de Melo @ 2020-07-30 10:03 UTC (permalink / raw) To: Ian Rogers Cc: Wang ShaoBo, cj.chengjian, Li Bin, Jiri Olsa, LKML, linux-perf-users Em Wed, Jul 29, 2020 at 04:47:36PM -0700, Ian Rogers escreveu: > On Fri, Jul 24, 2020 at 3:07 AM Wang ShaoBo <bobo.shaobowang@huawei.com> wrote: > > > > Function jvmti_write_code called by compiled_method_load_cb may return > > error in using fwrite_unlocked, this failure should be captured and > > warned. > > > > Signed-off-by: Wang ShaoBo <bobo.shaobowang@huawei.com> > > --- > > tools/perf/jvmti/jvmti_agent.c | 23 +++++++++++++++-------- > > 1 file changed, 15 insertions(+), 8 deletions(-) > > > > diff --git a/tools/perf/jvmti/jvmti_agent.c b/tools/perf/jvmti/jvmti_agent.c > > index 88108598d6e9..a1fe6aa16b6d 100644 > > --- a/tools/perf/jvmti/jvmti_agent.c > > +++ b/tools/perf/jvmti/jvmti_agent.c > > @@ -363,7 +363,7 @@ jvmti_write_code(void *agent, char const *sym, > > struct jr_code_load rec; > > size_t sym_len; > > FILE *fp = agent; > > - int ret = -1; > > + int ret; > > > > /* don't care about 0 length function, no samples */ > > if (size == 0) > > @@ -401,16 +401,23 @@ jvmti_write_code(void *agent, char const *sym, > > rec.code_index = code_generation++; > > > > ret = fwrite_unlocked(&rec, sizeof(rec), 1, fp); > > - fwrite_unlocked(sym, sym_len, 1, fp); > > + if (ret) > > + goto error; > > Sorry, it seems I lost a reply to this. Won't ret here be the number > of items written and not an error code? Consequently all writes will > immediately goto error? Yeah, I removed it from tmp.perf/core. Wang, please address Ian review, and consider having just one out exit label. - Arnaldo > Thanks, > Ian > > > + ret = fwrite_unlocked(sym, sym_len, 1, fp); > > + if (ret) > > + goto error; > > > > - if (code) > > - fwrite_unlocked(code, size, 1, fp); > > + if (code) { > > + ret = fwrite_unlocked(code, size, 1, fp); > > + if (ret) > > + goto error; > > + } > > > > funlockfile(fp); > > - > > - ret = 0; > > - > > - return ret; > > + return 0; > > +error: > > + funlockfile(fp); > > + return -1; > > } > > > > int > > -- > > 2.17.1 > > -- - Arnaldo ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH -next] tools build: Check return value of fwrite_unlocked in jvmti_agent.c 2020-07-30 10:03 ` Arnaldo Carvalho de Melo @ 2020-07-31 11:58 ` Wangshaobo (bobo) 0 siblings, 0 replies; 6+ messages in thread From: Wangshaobo (bobo) @ 2020-07-31 11:58 UTC (permalink / raw) To: Arnaldo Carvalho de Melo, Ian Rogers Cc: cj.chengjian, Li Bin, Jiri Olsa, LKML, linux-perf-users 在 2020/7/30 18:03, Arnaldo Carvalho de Melo 写道: > Em Wed, Jul 29, 2020 at 04:47:36PM -0700, Ian Rogers escreveu: >> On Fri, Jul 24, 2020 at 3:07 AM Wang ShaoBo <bobo.shaobowang@huawei.com> wrote: >>> Function jvmti_write_code called by compiled_method_load_cb may return >>> error in using fwrite_unlocked, this failure should be captured and >>> warned. >>> >>> Signed-off-by: Wang ShaoBo <bobo.shaobowang@huawei.com> >>> --- >>> tools/perf/jvmti/jvmti_agent.c | 23 +++++++++++++++-------- >>> 1 file changed, 15 insertions(+), 8 deletions(-) >>> >>> diff --git a/tools/perf/jvmti/jvmti_agent.c b/tools/perf/jvmti/jvmti_agent.c >>> index 88108598d6e9..a1fe6aa16b6d 100644 >>> --- a/tools/perf/jvmti/jvmti_agent.c >>> +++ b/tools/perf/jvmti/jvmti_agent.c >>> @@ -363,7 +363,7 @@ jvmti_write_code(void *agent, char const *sym, >>> struct jr_code_load rec; >>> size_t sym_len; >>> FILE *fp = agent; >>> - int ret = -1; >>> + int ret; >>> >>> /* don't care about 0 length function, no samples */ >>> if (size == 0) >>> @@ -401,16 +401,23 @@ jvmti_write_code(void *agent, char const *sym, >>> rec.code_index = code_generation++; >>> >>> ret = fwrite_unlocked(&rec, sizeof(rec), 1, fp); >>> - fwrite_unlocked(sym, sym_len, 1, fp); >>> + if (ret) >>> + goto error; >> Sorry, it seems I lost a reply to this. Won't ret here be the number >> of items written and not an error code? Consequently all writes will >> immediately goto error? > Yeah, I removed it from tmp.perf/core. > > Wang, please address Ian review, and consider having just one out exit > label. > > - Arnaldo > I am sorry I forgot what does fwrite_unlocked return, as same as fwrite error happens, the return val is a short item count (or zero) but not equals the items written. I have fixed it and already sent v2 and address Ian review, thanks a lot. ... and i refer to the writing of function jvmti_write_debug_info() for giving out exit label, thanks again, - Wang ShaoBo >> Thanks, >> Ian >> >>> + ret = fwrite_unlocked(sym, sym_len, 1, fp); >>> + if (ret) >>> + goto error; >>> >>> - if (code) >>> - fwrite_unlocked(code, size, 1, fp); >>> + if (code) { >>> + ret = fwrite_unlocked(code, size, 1, fp); >>> + if (ret) >>> + goto error; >>> + } >>> >>> funlockfile(fp); >>> - >>> - ret = 0; >>> - >>> - return ret; >>> + return 0; >>> +error: >>> + funlockfile(fp); >>> + return -1; >>> } >>> >>> int >>> -- >>> 2.17.1 >>> ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-07-31 11:58 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-07-24 10:07 [PATCH -next] tools build: Check return value of fwrite_unlocked in jvmti_agent.c Wang ShaoBo 2020-07-28 12:10 ` Arnaldo Carvalho de Melo 2020-07-29 23:47 ` Ian Rogers 2020-07-30 1:12 ` Arnaldo Carvalho de Melo 2020-07-30 10:03 ` Arnaldo Carvalho de Melo 2020-07-31 11:58 ` Wangshaobo (bobo)
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).