| Message ID | 20240912013846.3058728-1-peng.fan@oss.nxp.com |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] pm: cpupower: bench: print path fopen failed | expand |
Hello Peng, These two seem like two separate patches and usually a series has a cover letter. Did you mean to send them separately or is something missing?
On 9/11/24 19:38, Peng Fan (OSS) wrote: > From: Peng Fan <peng.fan@nxp.com> > > Print out the config file path when fopen failed. It will be easy > for users to know where to create the file. Send these two patches as a series with a cover letter. Also what is changing - you can include what change: use the same subject line in here. The subject line can be improved to say more than fopen() failed. Which file open failed? The message can be informative about which file: about which file. e.g: pm: cpupower: bench: print config file path when open fails > > Signed-off-by: Peng Fan <peng.fan@nxp.com> > --- > tools/power/cpupower/bench/parse.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/tools/power/cpupower/bench/parse.c b/tools/power/cpupower/bench/parse.c > index e63dc11fa3a5..366b20f9ddf1 100644 > --- a/tools/power/cpupower/bench/parse.c > +++ b/tools/power/cpupower/bench/parse.c > @@ -166,7 +166,7 @@ int prepare_config(const char *path, struct config *config) > configfile = fopen(path, "r"); > if (configfile == NULL) { > perror("fopen"); > - fprintf(stderr, "error: unable to read configfile\n"); > + fprintf(stderr, "error: unable to read configfile: %s\n", path); While you are at it, fix it to use strerror() instead of calling perror() followed by fprintf(). > free(config); > return 1; > } thanks, -- Shuah
> Subject: Re: [PATCH 1/2] pm: cpupower: bench: print path fopen failed > > On 9/11/24 19:38, Peng Fan (OSS) wrote: > > From: Peng Fan <peng.fan@nxp.com> > > > > Print out the config file path when fopen failed. It will be easy for > > users to know where to create the file. > > Send these two patches as a series with a cover letter. > > Also what is changing - you can include what change: use the same > subject line in here. > > The subject line can be improved to say more than fopen() failed. > Which file open failed? > > The message can be informative about which file: > about which file. > > e.g: pm: cpupower: bench: print config file path when open fails > > > > > Signed-off-by: Peng Fan <peng.fan@nxp.com> > > --- > > tools/power/cpupower/bench/parse.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/tools/power/cpupower/bench/parse.c > > b/tools/power/cpupower/bench/parse.c > > index e63dc11fa3a5..366b20f9ddf1 100644 > > --- a/tools/power/cpupower/bench/parse.c > > +++ b/tools/power/cpupower/bench/parse.c > > @@ -166,7 +166,7 @@ int prepare_config(const char *path, struct > config *config) > > configfile = fopen(path, "r"); > > if (configfile == NULL) { > > perror("fopen"); > > - fprintf(stderr, "error: unable to read configfile\n"); > > + fprintf(stderr, "error: unable to read configfile: %s\n", > path); > > While you are at it, fix it to use strerror() instead of calling perror() > followed by fprintf(). Seems the usage of perror is in the whole file. Could the conversion to sterror() be done in a separate patch? Thanks, Peng. > > > > free(config); > > return 1; > > } > > thanks, > -- Shuah
diff --git a/tools/power/cpupower/bench/parse.c b/tools/power/cpupower/bench/parse.c index e63dc11fa3a5..366b20f9ddf1 100644 --- a/tools/power/cpupower/bench/parse.c +++ b/tools/power/cpupower/bench/parse.c @@ -166,7 +166,7 @@ int prepare_config(const char *path, struct config *config) configfile = fopen(path, "r"); if (configfile == NULL) { perror("fopen"); - fprintf(stderr, "error: unable to read configfile\n"); + fprintf(stderr, "error: unable to read configfile: %s\n", path); free(config); return 1; }