* [PATCH v1 0/1] Fix mkimage error message
@ 2020-06-05 8:01 Mylène Josserand
2020-06-05 8:01 ` [PATCH v1 1/1] mkimage: Fix error message if write less data then expected Mylène Josserand
0 siblings, 1 reply; 6+ messages in thread
From: Mylène Josserand @ 2020-06-05 8:01 UTC (permalink / raw)
To: u-boot
Hello everyone,
While using mkimage with debos, I noticed a wierd error:
"mkimage: Write error on uImage: Success"
After some investications, I found that this error is in fact
a "No space left" error.
This patch is updating mkimage's code to print a new error
message in case the size written is less than the expected size.
We can't write all the data probably because of of not enough space
on the device.
Thank you in advance,
Best regards
Myl?ne
Myl?ne Josserand (1):
mkimage: Fix error message if write less data then expected
tools/mkimage.c | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)
--
2.26.2
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v1 1/1] mkimage: Fix error message if write less data then expected
2020-06-05 8:01 [PATCH v1 0/1] Fix mkimage error message Mylène Josserand
@ 2020-06-05 8:01 ` Mylène Josserand
2020-06-09 18:49 ` Walter Lozano
0 siblings, 1 reply; 6+ messages in thread
From: Mylène Josserand @ 2020-06-05 8:01 UTC (permalink / raw)
To: u-boot
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 <mylene.josserand@collabora.com>
---
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);
exit (EXIT_FAILURE);
}
--
2.26.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v1 1/1] mkimage: Fix error message if write less data then expected
2020-06-05 8:01 ` [PATCH v1 1/1] mkimage: Fix error message if write less data then expected Mylène Josserand
@ 2020-06-09 18:49 ` Walter Lozano
2020-07-07 8:25 ` Mylene Josserand
0 siblings, 1 reply; 6+ messages in thread
From: Walter Lozano @ 2020-06-09 18:49 UTC (permalink / raw)
To: u-boot
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 <mylene.josserand@collabora.com>
> ---
> 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.
> exit (EXIT_FAILURE);
> }
>
Regards,
Walter
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v1 1/1] mkimage: Fix error message if write less data then expected
2020-06-09 18:49 ` Walter Lozano
@ 2020-07-07 8:25 ` Mylene Josserand
2020-07-07 13:40 ` Walter Lozano
0 siblings, 1 reply; 6+ messages in thread
From: Mylene Josserand @ 2020-07-07 8:25 UTC (permalink / raw)
To: u-boot
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 <mylene.josserand@collabora.com>
>> ---
>> ? 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.
>
>> ????????? exit (EXIT_FAILURE);
>> ????? }
>
>
> Regards,
>
> Walter
>
Best regards,
Myl?ne
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v1 1/1] mkimage: Fix error message if write less data then expected
2020-07-07 8:25 ` Mylene Josserand
@ 2020-07-07 13:40 ` Walter Lozano
2020-07-08 8:50 ` Mylene Josserand
0 siblings, 1 reply; 6+ messages in thread
From: Walter Lozano @ 2020-07-07 13:40 UTC (permalink / raw)
To: u-boot
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 <mylene.josserand@collabora.com>
>>> ---
>>> ? 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 <walter.lozano@collabora.com>
>
>>
>>> ????????? exit (EXIT_FAILURE);
>>> ????? }
>>
Regards,
Walter
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v1 1/1] mkimage: Fix error message if write less data then expected
2020-07-07 13:40 ` Walter Lozano
@ 2020-07-08 8:50 ` Mylene Josserand
0 siblings, 0 replies; 6+ messages in thread
From: Mylene Josserand @ 2020-07-08 8:50 UTC (permalink / raw)
To: u-boot
Hello,
On 7/7/20 3:40 PM, Walter Lozano wrote:
> 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 <mylene.josserand@collabora.com>
>>>> ---
>>>> ? 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?
Yes, why not. I thought that "No space left" is a more common error
message but let's change that with more details about how many
written/expected bytes.
>
> Besides that
>
> Reviewed-by: Walter Lozano <walter.lozano@collabora.com>
Thanks!
>
>>
>>>
>>>> ????????? exit (EXIT_FAILURE);
>>>> ????? }
>>>
> Regards,
>
> Walter
>
Best regards,
Myl?ne
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-07-08 8:50 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-05 8:01 [PATCH v1 0/1] Fix mkimage error message Mylène Josserand
2020-06-05 8:01 ` [PATCH v1 1/1] mkimage: Fix error message if write less data then expected Mylène Josserand
2020-06-09 18:49 ` Walter Lozano
2020-07-07 8:25 ` Mylene Josserand
2020-07-07 13:40 ` Walter Lozano
2020-07-08 8:50 ` Mylene Josserand
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.