[PATCH v1 1/1] mkimage: Fix error message if write less data then expected

[Mylene Josserand]

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 at 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 at collabora.com>

Thanks!

> 
>>
>>>
>>>>           exit (EXIT_FAILURE);
>>>>       }
>>>
> Regards,
> 
> Walter
> 

Best regards,
Mylène