From mboxrd@z Thu Jan 1 00:00:00 1970 From: Walter Lozano Date: Tue, 7 Jul 2020 10:40:28 -0300 Subject: [PATCH v1 1/1] mkimage: Fix error message if write less data then expected In-Reply-To: References: <20200605080127.10156-1-mylene.josserand@collabora.com> <20200605080127.10156-2-mylene.josserand@collabora.com> Message-ID: <2282215c-ca76-ba38-8229-d5e2ec9ac77e@collabora.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Mylene, On 7/7/20 05:25, Mylene Josserand wrote: > Hi, > > On 6/9/20 8:49 PM, Walter Lozano wrote: >> Hi Mylene, >> >> Thanks for you patch. >> >> On 5/6/20 05:01, Myl?ne Josserand wrote: >>> Add a new error message in case the size of data written >>> are shorter than the one expected. >>> >>> Currently, it will lead to the following error message: >>> ??? "mkimage: Write error on uImage: Success" >>> >>> This is not explicit when the error is because the device >>> doesn't have enough space. Let's use a "No space left on >>> the device" error message in that case. >>> >>> Signed-off-by: Myl?ne Josserand >>> --- >>> ? tools/mkimage.c | 14 ++++++++++---- >>> ? 1 file changed, 10 insertions(+), 4 deletions(-) >>> >>> diff --git a/tools/mkimage.c b/tools/mkimage.c >>> index d2cd1917874..ef0cc889a92 100644 >>> --- a/tools/mkimage.c >>> +++ b/tools/mkimage.c >>> @@ -674,7 +674,7 @@ copy_file (int ifd, const char *datafile, int pad) >>> ????? int zero = 0; >>> ????? uint8_t zeros[4096]; >>> ????? int offset = 0; >>> -??? int size; >>> +??? int size, ret; >>> ????? struct image_type_params *tparams = >>> imagetool_get_type(params.type); >>> ????? memset(zeros, 0, sizeof(zeros)); >>> @@ -730,9 +730,15 @@ copy_file (int ifd, const char *datafile, int pad) >>> ????? } >>> ????? size = sbuf.st_size - offset; >>> -??? if (write(ifd, ptr + offset, size) != size) { >>> -??????? fprintf (stderr, "%s: Write error on %s: %s\n", >>> -??????????? params.cmdname, params.imagefile, strerror(errno)); >>> + >>> +??? ret = write(ifd, ptr + offset, size); >>> +??? if (ret != size) { >>> +??????? if (ret < 0) >>> +??????????? fprintf (stderr, "%s: Write error on %s: %s\n", >>> +???????????????? params.cmdname, params.imagefile, strerror(errno)); >>> +??????? else if (ret < size) >>> +??????????? fprintf (stderr, "%s: No space left on the device\n", >>> +???????????????? params.cmdname); >> >> >> Thanks for improving the error message, it is much more clear now. >> The only question I have is if "no space left on the device" is the >> only possible cause for this situation, for sure it is the most >> probable. > > Thanks for your review. Indeed, it is the most probable I guess. Yes, I agree. However, in order to be more accurate, and probably avoid future patches to correct a misleading message I would improve the error message with something like "Fewer bytes written than expected, probably no space left on the device" or something similar. What do you think? Besides that Reviewed-by: Walter Lozano > >> >>> ????????? exit (EXIT_FAILURE); >>> ????? } >> Regards, Walter