diff mbox series

[v9,01/11] tools: mkeficapsule: output messages to stderr instead of stdout

Message ID 20220118043954.55940-2-takahiro.akashi@linaro.org
State Accepted, archived
Delegated to: Heinrich Schuchardt
Headers show
Series efi_loader: capsule: improve capsule authentication support | expand

Commit Message

AKASHI Takahiro Jan. 18, 2022, 4:39 a.m. UTC 
All the error messages should be printed out to stderr.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 tools/mkeficapsule.c | 35 ++++++++++++++++++-----------------
 1 file changed, 18 insertions(+), 17 deletions(-)

Comments

Heinrich Schuchardt Jan. 19, 2022, 3:41 p.m. UTC | #1 
On 1/18/22 05:39, AKASHI Takahiro wrote:
> All the error messages should be printed out to stderr.
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
>   tools/mkeficapsule.c | 35 ++++++++++++++++++-----------------
>   1 file changed, 18 insertions(+), 17 deletions(-)
>
> diff --git a/tools/mkeficapsule.c b/tools/mkeficapsule.c
> index 4995ba4e0c2a..19d5eea3cb59 100644
> --- a/tools/mkeficapsule.c
> +++ b/tools/mkeficapsule.c
> @@ -50,7 +50,7 @@ static struct option options[] = {
>
>   static void print_usage(void)
>   {
> -	printf("Usage: %s [options] <output file>\n"
> +	fprintf(stderr, "Usage: %s [options] <output file>\n"

Using mkimage -h is not an error condition. I will remove this hunk when
merging.

>   	       "Options:\n"
>
>   	       "\t-f, --fit <fit image>       new FIT image file\n"
> @@ -74,28 +74,29 @@ static int create_fwbin(char *path, char *bin, efi_guid_t *guid,
>   	u64 offset;
>
>   #ifdef DEBUG
> -	printf("For output: %s\n", path);
> -	printf("\tbin: %s\n\ttype: %pUl\n", bin, guid);
> -	printf("\tindex: %ld\n\tinstance: %ld\n", index, instance);
> +	fprintf(stderr, "For output: %s\n", path);
> +	fprintf(stderr, "\tbin: %s\n\ttype: %pUl\n", bin, guid);
> +	fprintf(stderr, "\tindex: %ld\n\tinstance: %ld\n", index, instance);

This is not an error. I will revert these lines when merging.

Otherwise

Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>

>   #endif
>
>   	g = fopen(bin, "r");
>   	if (!g) {
> -		printf("cannot open %s\n", bin);
> +		fprintf(stderr, "cannot open %s\n", bin);
>   		return -1;
>   	}
>   	if (stat(bin, &bin_stat) < 0) {
> -		printf("cannot determine the size of %s\n", bin);
> +		fprintf(stderr, "cannot determine the size of %s\n", bin);
>   		goto err_1;
>   	}
>   	data = malloc(bin_stat.st_size);
>   	if (!data) {
> -		printf("cannot allocate memory: %zx\n", (size_t)bin_stat.st_size);
> +		fprintf(stderr, "cannot allocate memory: %zx\n",
> +			(size_t)bin_stat.st_size);
>   		goto err_1;
>   	}
>   	f = fopen(path, "w");
>   	if (!f) {
> -		printf("cannot open %s\n", path);
> +		fprintf(stderr, "cannot open %s\n", path);
>   		goto err_2;
>   	}
>   	header.capsule_guid = efi_guid_fm_capsule;
> @@ -109,7 +110,7 @@ static int create_fwbin(char *path, char *bin, efi_guid_t *guid,
>
>   	size = fwrite(&header, 1, sizeof(header), f);
>   	if (size < sizeof(header)) {
> -		printf("write failed (%zx)\n", size);
> +		fprintf(stderr, "write failed (%zx)\n", size);
>   		goto err_3;
>   	}
>
> @@ -118,13 +119,13 @@ static int create_fwbin(char *path, char *bin, efi_guid_t *guid,
>   	capsule.payload_item_count = 1;
>   	size = fwrite(&capsule, 1, sizeof(capsule), f);
>   	if (size < (sizeof(capsule))) {
> -		printf("write failed (%zx)\n", size);
> +		fprintf(stderr, "write failed (%zx)\n", size);
>   		goto err_3;
>   	}
>   	offset = sizeof(capsule) + sizeof(u64);
>   	size = fwrite(&offset, 1, sizeof(offset), f);
>   	if (size < sizeof(offset)) {
> -		printf("write failed (%zx)\n", size);
> +		fprintf(stderr, "write failed (%zx)\n", size);
>   		goto err_3;
>   	}
>
> @@ -141,17 +142,17 @@ static int create_fwbin(char *path, char *bin, efi_guid_t *guid,
>
>   	size = fwrite(&image, 1, sizeof(image), f);
>   	if (size < sizeof(image)) {
> -		printf("write failed (%zx)\n", size);
> +		fprintf(stderr, "write failed (%zx)\n", size);
>   		goto err_3;
>   	}
>   	size = fread(data, 1, bin_stat.st_size, g);
>   	if (size < bin_stat.st_size) {
> -		printf("read failed (%zx)\n", size);
> +		fprintf(stderr, "read failed (%zx)\n", size);
>   		goto err_3;
>   	}
>   	size = fwrite(data, 1, bin_stat.st_size, f);
>   	if (size < bin_stat.st_size) {
> -		printf("write failed (%zx)\n", size);
> +		fprintf(stderr, "write failed (%zx)\n", size);
>   		goto err_3;
>   	}
>
> @@ -194,7 +195,7 @@ int main(int argc, char **argv)
>   		switch (c) {
>   		case 'f':
>   			if (file) {
> -				printf("Image already specified\n");
> +				fprintf(stderr, "Image already specified\n");
>   				return -1;
>   			}
>   			file = optarg;
> @@ -202,7 +203,7 @@ int main(int argc, char **argv)
>   			break;
>   		case 'r':
>   			if (file) {
> -				printf("Image already specified\n");
> +				fprintf(stderr, "Image already specified\n");
>   				return -1;
>   			}
>   			file = optarg;
> @@ -234,7 +235,7 @@ int main(int argc, char **argv)
>
>   	if (create_fwbin(argv[optind], file, guid, index, instance)
>   			< 0) {
> -		printf("Creating firmware capsule failed\n");
> +		fprintf(stderr, "Creating firmware capsule failed\n");
>   		exit(EXIT_FAILURE);
>   	}
>
AKASHI Takahiro Jan. 20, 2022, 5:58 a.m. UTC | #2 
On Wed, Jan 19, 2022 at 04:41:15PM +0100, Heinrich Schuchardt wrote:
> On 1/18/22 05:39, AKASHI Takahiro wrote:
> > All the error messages should be printed out to stderr.
> > 
> > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > ---
> >   tools/mkeficapsule.c | 35 ++++++++++++++++++-----------------
> >   1 file changed, 18 insertions(+), 17 deletions(-)
> > 
> > diff --git a/tools/mkeficapsule.c b/tools/mkeficapsule.c
> > index 4995ba4e0c2a..19d5eea3cb59 100644
> > --- a/tools/mkeficapsule.c
> > +++ b/tools/mkeficapsule.c
> > @@ -50,7 +50,7 @@ static struct option options[] = {
> > 
> >   static void print_usage(void)
> >   {
> > -	printf("Usage: %s [options] <output file>\n"
> > +	fprintf(stderr, "Usage: %s [options] <output file>\n"
> 
> Using mkimage -h is not an error condition. I will remove this hunk when
> merging.

A help message will be shown not only by "-h" but also in case that
invalid combination of parameters are passed.
In this sense, it is part of error messages.

Almost of all other tools under tools/ also behave
in the same way.

-Takahiro Akashi


> >   	       "Options:\n"
> > 
> >   	       "\t-f, --fit <fit image>       new FIT image file\n"
> > @@ -74,28 +74,29 @@ static int create_fwbin(char *path, char *bin, efi_guid_t *guid,
> >   	u64 offset;
> > 
> >   #ifdef DEBUG
> > -	printf("For output: %s\n", path);
> > -	printf("\tbin: %s\n\ttype: %pUl\n", bin, guid);
> > -	printf("\tindex: %ld\n\tinstance: %ld\n", index, instance);
> > +	fprintf(stderr, "For output: %s\n", path);
> > +	fprintf(stderr, "\tbin: %s\n\ttype: %pUl\n", bin, guid);
> > +	fprintf(stderr, "\tindex: %ld\n\tinstance: %ld\n", index, instance);
> 
> This is not an error. I will revert these lines when merging.
> 
> Otherwise
> 
> Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> 
> >   #endif
> > 
> >   	g = fopen(bin, "r");
> >   	if (!g) {
> > -		printf("cannot open %s\n", bin);
> > +		fprintf(stderr, "cannot open %s\n", bin);
> >   		return -1;
> >   	}
> >   	if (stat(bin, &bin_stat) < 0) {
> > -		printf("cannot determine the size of %s\n", bin);
> > +		fprintf(stderr, "cannot determine the size of %s\n", bin);
> >   		goto err_1;
> >   	}
> >   	data = malloc(bin_stat.st_size);
> >   	if (!data) {
> > -		printf("cannot allocate memory: %zx\n", (size_t)bin_stat.st_size);
> > +		fprintf(stderr, "cannot allocate memory: %zx\n",
> > +			(size_t)bin_stat.st_size);
> >   		goto err_1;
> >   	}
> >   	f = fopen(path, "w");
> >   	if (!f) {
> > -		printf("cannot open %s\n", path);
> > +		fprintf(stderr, "cannot open %s\n", path);
> >   		goto err_2;
> >   	}
> >   	header.capsule_guid = efi_guid_fm_capsule;
> > @@ -109,7 +110,7 @@ static int create_fwbin(char *path, char *bin, efi_guid_t *guid,
> > 
> >   	size = fwrite(&header, 1, sizeof(header), f);
> >   	if (size < sizeof(header)) {
> > -		printf("write failed (%zx)\n", size);
> > +		fprintf(stderr, "write failed (%zx)\n", size);
> >   		goto err_3;
> >   	}
> > 
> > @@ -118,13 +119,13 @@ static int create_fwbin(char *path, char *bin, efi_guid_t *guid,
> >   	capsule.payload_item_count = 1;
> >   	size = fwrite(&capsule, 1, sizeof(capsule), f);
> >   	if (size < (sizeof(capsule))) {
> > -		printf("write failed (%zx)\n", size);
> > +		fprintf(stderr, "write failed (%zx)\n", size);
> >   		goto err_3;
> >   	}
> >   	offset = sizeof(capsule) + sizeof(u64);
> >   	size = fwrite(&offset, 1, sizeof(offset), f);
> >   	if (size < sizeof(offset)) {
> > -		printf("write failed (%zx)\n", size);
> > +		fprintf(stderr, "write failed (%zx)\n", size);
> >   		goto err_3;
> >   	}
> > 
> > @@ -141,17 +142,17 @@ static int create_fwbin(char *path, char *bin, efi_guid_t *guid,
> > 
> >   	size = fwrite(&image, 1, sizeof(image), f);
> >   	if (size < sizeof(image)) {
> > -		printf("write failed (%zx)\n", size);
> > +		fprintf(stderr, "write failed (%zx)\n", size);
> >   		goto err_3;
> >   	}
> >   	size = fread(data, 1, bin_stat.st_size, g);
> >   	if (size < bin_stat.st_size) {
> > -		printf("read failed (%zx)\n", size);
> > +		fprintf(stderr, "read failed (%zx)\n", size);
> >   		goto err_3;
> >   	}
> >   	size = fwrite(data, 1, bin_stat.st_size, f);
> >   	if (size < bin_stat.st_size) {
> > -		printf("write failed (%zx)\n", size);
> > +		fprintf(stderr, "write failed (%zx)\n", size);
> >   		goto err_3;
> >   	}
> > 
> > @@ -194,7 +195,7 @@ int main(int argc, char **argv)
> >   		switch (c) {
> >   		case 'f':
> >   			if (file) {
> > -				printf("Image already specified\n");
> > +				fprintf(stderr, "Image already specified\n");
> >   				return -1;
> >   			}
> >   			file = optarg;
> > @@ -202,7 +203,7 @@ int main(int argc, char **argv)
> >   			break;
> >   		case 'r':
> >   			if (file) {
> > -				printf("Image already specified\n");
> > +				fprintf(stderr, "Image already specified\n");
> >   				return -1;
> >   			}
> >   			file = optarg;
> > @@ -234,7 +235,7 @@ int main(int argc, char **argv)
> > 
> >   	if (create_fwbin(argv[optind], file, guid, index, instance)
> >   			< 0) {
> > -		printf("Creating firmware capsule failed\n");
> > +		fprintf(stderr, "Creating firmware capsule failed\n");
> >   		exit(EXIT_FAILURE);
> >   	}
> > 
>
diff mbox series

Patch

diff --git a/tools/mkeficapsule.c b/tools/mkeficapsule.c
index 4995ba4e0c2a..19d5eea3cb59 100644
--- a/tools/mkeficapsule.c
+++ b/tools/mkeficapsule.c
@@ -50,7 +50,7 @@  static struct option options[] = {
 
 static void print_usage(void)
 {
-	printf("Usage: %s [options] <output file>\n"
+	fprintf(stderr, "Usage: %s [options] <output file>\n"
 	       "Options:\n"
 
 	       "\t-f, --fit <fit image>       new FIT image file\n"
@@ -74,28 +74,29 @@  static int create_fwbin(char *path, char *bin, efi_guid_t *guid,
 	u64 offset;
 
 #ifdef DEBUG
-	printf("For output: %s\n", path);
-	printf("\tbin: %s\n\ttype: %pUl\n", bin, guid);
-	printf("\tindex: %ld\n\tinstance: %ld\n", index, instance);
+	fprintf(stderr, "For output: %s\n", path);
+	fprintf(stderr, "\tbin: %s\n\ttype: %pUl\n", bin, guid);
+	fprintf(stderr, "\tindex: %ld\n\tinstance: %ld\n", index, instance);
 #endif
 
 	g = fopen(bin, "r");
 	if (!g) {
-		printf("cannot open %s\n", bin);
+		fprintf(stderr, "cannot open %s\n", bin);
 		return -1;
 	}
 	if (stat(bin, &bin_stat) < 0) {
-		printf("cannot determine the size of %s\n", bin);
+		fprintf(stderr, "cannot determine the size of %s\n", bin);
 		goto err_1;
 	}
 	data = malloc(bin_stat.st_size);
 	if (!data) {
-		printf("cannot allocate memory: %zx\n", (size_t)bin_stat.st_size);
+		fprintf(stderr, "cannot allocate memory: %zx\n",
+			(size_t)bin_stat.st_size);
 		goto err_1;
 	}
 	f = fopen(path, "w");
 	if (!f) {
-		printf("cannot open %s\n", path);
+		fprintf(stderr, "cannot open %s\n", path);
 		goto err_2;
 	}
 	header.capsule_guid = efi_guid_fm_capsule;
@@ -109,7 +110,7 @@  static int create_fwbin(char *path, char *bin, efi_guid_t *guid,
 
 	size = fwrite(&header, 1, sizeof(header), f);
 	if (size < sizeof(header)) {
-		printf("write failed (%zx)\n", size);
+		fprintf(stderr, "write failed (%zx)\n", size);
 		goto err_3;
 	}
 
@@ -118,13 +119,13 @@  static int create_fwbin(char *path, char *bin, efi_guid_t *guid,
 	capsule.payload_item_count = 1;
 	size = fwrite(&capsule, 1, sizeof(capsule), f);
 	if (size < (sizeof(capsule))) {
-		printf("write failed (%zx)\n", size);
+		fprintf(stderr, "write failed (%zx)\n", size);
 		goto err_3;
 	}
 	offset = sizeof(capsule) + sizeof(u64);
 	size = fwrite(&offset, 1, sizeof(offset), f);
 	if (size < sizeof(offset)) {
-		printf("write failed (%zx)\n", size);
+		fprintf(stderr, "write failed (%zx)\n", size);
 		goto err_3;
 	}
 
@@ -141,17 +142,17 @@  static int create_fwbin(char *path, char *bin, efi_guid_t *guid,
 
 	size = fwrite(&image, 1, sizeof(image), f);
 	if (size < sizeof(image)) {
-		printf("write failed (%zx)\n", size);
+		fprintf(stderr, "write failed (%zx)\n", size);
 		goto err_3;
 	}
 	size = fread(data, 1, bin_stat.st_size, g);
 	if (size < bin_stat.st_size) {
-		printf("read failed (%zx)\n", size);
+		fprintf(stderr, "read failed (%zx)\n", size);
 		goto err_3;
 	}
 	size = fwrite(data, 1, bin_stat.st_size, f);
 	if (size < bin_stat.st_size) {
-		printf("write failed (%zx)\n", size);
+		fprintf(stderr, "write failed (%zx)\n", size);
 		goto err_3;
 	}
 
@@ -194,7 +195,7 @@  int main(int argc, char **argv)
 		switch (c) {
 		case 'f':
 			if (file) {
-				printf("Image already specified\n");
+				fprintf(stderr, "Image already specified\n");
 				return -1;
 			}
 			file = optarg;
@@ -202,7 +203,7 @@  int main(int argc, char **argv)
 			break;
 		case 'r':
 			if (file) {
-				printf("Image already specified\n");
+				fprintf(stderr, "Image already specified\n");
 				return -1;
 			}
 			file = optarg;
@@ -234,7 +235,7 @@  int main(int argc, char **argv)
 
 	if (create_fwbin(argv[optind], file, guid, index, instance)
 			< 0) {
-		printf("Creating firmware capsule failed\n");
+		fprintf(stderr, "Creating firmware capsule failed\n");
 		exit(EXIT_FAILURE);
 	}