From 57a608e9694045c84ef63878c2bb57a30a0c6800 Mon Sep 17 00:00:00 2001 From: Martyn Welch Date: Sat, 26 Jan 2019 02:31:50 +0000 Subject: tools: dumpimage: Provide more feedback on error The dumpimage utility errors out in a number of places without providing sufficient feedback to allow the user to easily determine what they have done wrong. Add addtional error messages to make the cause of the failure more obvious. Signed-off-by: Martyn Welch --- tools/dumpimage.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) (limited to 'tools') diff --git a/tools/dumpimage.c b/tools/dumpimage.c index 7115df04c12..2847e6c0b47 100644 --- a/tools/dumpimage.c +++ b/tools/dumpimage.c @@ -80,6 +80,8 @@ int main(int argc, char **argv) case 'T': params.type = genimg_get_type_id(optarg); if (params.type < 0) { + fprintf(stderr, "%s: Invalid type\n", + params.cmdname); usage(); } break; @@ -101,8 +103,10 @@ int main(int argc, char **argv) } } - if (optind >= argc) + if (optind >= argc) { + fprintf(stderr, "%s: image file missing\n", params.cmdname); usage(); + } /* set tparams as per input type_id */ tparams = imagetool_get_type(params.type); @@ -117,8 +121,11 @@ int main(int argc, char **argv) * as per image type to be generated/listed */ if (tparams->check_params) { - if (tparams->check_params(¶ms)) + if (tparams->check_params(¶ms)) { + fprintf(stderr, "%s: Parameter check failed\n", + params.cmdname); usage(); + } } if (params.iflag) -- cgit v1.3.1 From 12b831879a765722c1a94ca75c6adb6f80759cd9 Mon Sep 17 00:00:00 2001 From: Martyn Welch Date: Sat, 26 Jan 2019 02:31:51 +0000 Subject: tools: dumpimage: Simplify arguments The dump image utility has very confusing syntax. If called to list image contents ("-l") it takes the image name as a positional argument. If the utility is called to extract something from the image, the image must be provided via the optional argument "-i" as well as the positional argument but the value passed in the positional argument will be completely ignored. Simplify dumpimage by always providing the image as the first positional argument. Assume we want to dump something from the image if we do not provide the "-l" option for now. Signed-off-by: Martyn Welch --- tools/dumpimage.c | 25 +++++++++++-------------- 1 file changed, 11 insertions(+), 14 deletions(-) (limited to 'tools') diff --git a/tools/dumpimage.c b/tools/dumpimage.c index 2847e6c0b47..1b92dec364e 100644 --- a/tools/dumpimage.c +++ b/tools/dumpimage.c @@ -65,17 +65,14 @@ int main(int argc, char **argv) params.cmdname = *argv; - while ((opt = getopt(argc, argv, "li:o:T:p:V")) != -1) { + while ((opt = getopt(argc, argv, "lo:T:p:V")) != -1) { switch (opt) { case 'l': params.lflag = 1; break; - case 'i': - params.imagefile = optarg; - params.iflag = 1; - break; case 'o': params.outfile = optarg; + params.iflag = 1; break; case 'T': params.type = genimg_get_type_id(optarg); @@ -108,6 +105,8 @@ int main(int argc, char **argv) usage(); } + params.imagefile = argv[optind]; + /* set tparams as per input type_id */ tparams = imagetool_get_type(params.type); if (tparams == NULL) { @@ -128,12 +127,11 @@ int main(int argc, char **argv) } } - if (params.iflag) - params.datafile = argv[optind]; - else - params.imagefile = argv[optind]; - if (!params.outfile) - params.outfile = params.datafile; + if (!params.lflag && !params.outfile) { + fprintf(stderr, "%s: No output file provided\n", + params.cmdname); + exit(EXIT_FAILURE); + } ifd = open(params.imagefile, O_RDONLY|O_BINARY); if (ifd < 0) { @@ -204,10 +202,9 @@ static void usage(void) " -l ==> list image header information\n", params.cmdname); fprintf(stderr, - " %s -i image -T type [-p position] [-o outfile] data_file\n" - " -i ==> extract from the 'image' a specific 'data_file'\n" + " %s -T type [-p position] [-o outfile] image\n" " -T ==> set image type to 'type'\n" - " -p ==> 'position' (starting at 0) of the 'data_file' inside the 'image'\n", + " -p ==> 'position' (starting at 0) of the component to extract from image\n", params.cmdname); fprintf(stderr, " %s -V ==> print version information and exit\n", -- cgit v1.3.1 From 11e3c1fd3bd8fd0a91d502fa4742749f877867bb Mon Sep 17 00:00:00 2001 From: Martyn Welch Date: Sat, 26 Jan 2019 02:31:52 +0000 Subject: tools: dumpimage: Simplify internal logic There are 3 supported modes of operation: 1) Show version 2) List image contents 3) Extract image component Option (1) terminates early, so only options (2) and (3) remain. Remove redundant check for these modes. Signed-off-by: Martyn Welch --- tools/dumpimage.c | 85 ++++++++++++++++++++++++------------------------------- 1 file changed, 37 insertions(+), 48 deletions(-) (limited to 'tools') diff --git a/tools/dumpimage.c b/tools/dumpimage.c index 1b92dec364e..e17e979b04d 100644 --- a/tools/dumpimage.c +++ b/tools/dumpimage.c @@ -60,7 +60,7 @@ int main(int argc, char **argv) int ifd = -1; struct stat sbuf; char *ptr; - int retval = 0; + int retval = EXIT_SUCCESS; struct image_type_params *tparams = NULL; params.cmdname = *argv; @@ -135,65 +135,54 @@ int main(int argc, char **argv) ifd = open(params.imagefile, O_RDONLY|O_BINARY); if (ifd < 0) { - fprintf(stderr, "%s: Can't open \"%s\": %s\n", - params.cmdname, params.imagefile, - strerror(errno)); + fprintf(stderr, "%s: Can't open \"%s\": %s\n", params.cmdname, + params.imagefile, strerror(errno)); exit(EXIT_FAILURE); } - if (params.lflag || params.iflag) { - if (fstat(ifd, &sbuf) < 0) { - fprintf(stderr, "%s: Can't stat \"%s\": %s\n", - params.cmdname, params.imagefile, - strerror(errno)); - exit(EXIT_FAILURE); - } + if (fstat(ifd, &sbuf) < 0) { + fprintf(stderr, "%s: Can't stat \"%s\": %s\n", params.cmdname, + params.imagefile, strerror(errno)); + exit(EXIT_FAILURE); + } - if ((uint32_t)sbuf.st_size < tparams->header_size) { - fprintf(stderr, - "%s: Bad size: \"%s\" is not valid image\n", - params.cmdname, params.imagefile); - exit(EXIT_FAILURE); - } + if ((uint32_t)sbuf.st_size < tparams->header_size) { + fprintf(stderr, "%s: Bad size: \"%s\" is not valid image\n", + params.cmdname, params.imagefile); + exit(EXIT_FAILURE); + } - ptr = mmap(0, sbuf.st_size, PROT_READ, MAP_SHARED, ifd, 0); - if (ptr == MAP_FAILED) { - fprintf(stderr, "%s: Can't read \"%s\": %s\n", - params.cmdname, params.imagefile, - strerror(errno)); - exit(EXIT_FAILURE); - } + ptr = mmap(0, sbuf.st_size, PROT_READ, MAP_SHARED, ifd, 0); + if (ptr == MAP_FAILED) { + fprintf(stderr, "%s: Can't read \"%s\": %s\n", params.cmdname, + params.imagefile, strerror(errno)); + exit(EXIT_FAILURE); + } + /* + * Both calls bellow scan through dumpimage registry for all + * supported image types and verify the input image file + * header for match + */ + if (params.iflag) { /* - * Both calls bellow scan through dumpimage registry for all - * supported image types and verify the input image file - * header for match + * Extract the data files from within the matched + * image type. Returns the error code if not matched */ - if (params.iflag) { - /* - * Extract the data files from within the matched - * image type. Returns the error code if not matched - */ - retval = dumpimage_extract_subimage(tparams, ptr, - &sbuf); - } else { - /* - * Print the image information for matched image type - * Returns the error code if not matched - */ - retval = imagetool_verify_print_header(ptr, &sbuf, - tparams, ¶ms); - } - - (void)munmap((void *)ptr, sbuf.st_size); - (void)close(ifd); - - return retval; + retval = dumpimage_extract_subimage(tparams, ptr, &sbuf); + } else { + /* + * Print the image information for matched image type + * Returns the error code if not matched + */ + retval = imagetool_verify_print_header(ptr, &sbuf, tparams, + ¶ms); } + (void)munmap((void *)ptr, sbuf.st_size); (void)close(ifd); - return EXIT_SUCCESS; + return retval; } static void usage(void) -- cgit v1.3.1 From 65a80b43b308bfae0b732765f75c25e5f65b9d84 Mon Sep 17 00:00:00 2001 From: Martyn Welch Date: Sat, 26 Jan 2019 02:31:53 +0000 Subject: tools: dumpimage: Add help option and make error paths consistent The utility dumpimage has error paths that display the usage and others that exit without displaying usage. Add an explicit help option to dumpimage to display the usage and remove it's use in error paths to make the error messages more obvious and errors paths more consistent. Signed-off-by: Martyn Welch --- tools/dumpimage.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) (limited to 'tools') diff --git a/tools/dumpimage.c b/tools/dumpimage.c index e17e979b04d..5c9ad363222 100644 --- a/tools/dumpimage.c +++ b/tools/dumpimage.c @@ -65,7 +65,7 @@ int main(int argc, char **argv) params.cmdname = *argv; - while ((opt = getopt(argc, argv, "lo:T:p:V")) != -1) { + while ((opt = getopt(argc, argv, "hlo:T:p:V")) != -1) { switch (opt) { case 'l': params.lflag = 1; @@ -79,7 +79,7 @@ int main(int argc, char **argv) if (params.type < 0) { fprintf(stderr, "%s: Invalid type\n", params.cmdname); - usage(); + exit(EXIT_FAILURE); } break; case 'p': @@ -94,15 +94,20 @@ int main(int argc, char **argv) case 'V': printf("dumpimage version %s\n", PLAIN_VERSION); exit(EXIT_SUCCESS); + case 'h': + usage(); default: usage(); break; } } + if (argc < 2) + usage(); + if (optind >= argc) { fprintf(stderr, "%s: image file missing\n", params.cmdname); - usage(); + exit(EXIT_FAILURE); } params.imagefile = argv[optind]; @@ -123,7 +128,7 @@ int main(int argc, char **argv) if (tparams->check_params(¶ms)) { fprintf(stderr, "%s: Parameter check failed\n", params.cmdname); - usage(); + exit(EXIT_FAILURE); } } @@ -195,9 +200,12 @@ static void usage(void) " -T ==> set image type to 'type'\n" " -p ==> 'position' (starting at 0) of the component to extract from image\n", params.cmdname); + fprintf(stderr, + " %s -h ==> print usage information and exit\n", + params.cmdname); fprintf(stderr, " %s -V ==> print version information and exit\n", params.cmdname); - exit(EXIT_FAILURE); + exit(EXIT_SUCCESS); } -- cgit v1.3.1 From e3b4fc9598388f47632a8c802aaa68b1154526f2 Mon Sep 17 00:00:00 2001 From: Martyn Welch Date: Sat, 26 Jan 2019 02:31:54 +0000 Subject: tools: dumpimage: Clarify help Help message isn't clear over the use of the "-T" option (it's to declare the type of image that the tool is operating on), which also is optional as it defaults to the default image type. It's also missing a description of the "-o" option, so add it. Signed-off-by: Martyn Welch --- tools/dumpimage.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) (limited to 'tools') diff --git a/tools/dumpimage.c b/tools/dumpimage.c index 5c9ad363222..ee3d41dda4d 100644 --- a/tools/dumpimage.c +++ b/tools/dumpimage.c @@ -196,9 +196,10 @@ static void usage(void) " -l ==> list image header information\n", params.cmdname); fprintf(stderr, - " %s -T type [-p position] [-o outfile] image\n" - " -T ==> set image type to 'type'\n" - " -p ==> 'position' (starting at 0) of the component to extract from image\n", + " %s [-T type] [-p position] [-o outfile] image\n" + " -T ==> declare image type as 'type'\n" + " -p ==> 'position' (starting at 0) of the component to extract from image\n" + " -o ==> extract component to file 'outfile'\n", params.cmdname); fprintf(stderr, " %s -h ==> print usage information and exit\n", -- cgit v1.3.1