Re: [PATCH 1/2] HABv4: remove useless error message

[Marc Kleine-Budde <mkl@pengutronix.de>]

[-- Attachment #1.1.1: Type: text/plain, Size: 3834 bytes --]

On 12/3/19 3:36 PM, Sascha Hauer wrote:
>> I think there is just one warning in the buffer:
>>
>>> HABv4: Status: Operation completed with warning (0x69)
>>> HABv4: Config: Secure IC (0xcc)
>>> HABv4: State: Trusted state (0x99)
>>> HABv4: ERROR: Recompile with larger event data buffer (at least 36 bytes)
>>
>> The barebox producing that output is missing both patches:
>>
>> 81e2b508e785 i.MX habv4: habv4_get_status(): display warning events, too
>> e7c33540d0c0 i.MX: HABv4: Reset index variable after error type
>>
>> The question is, do we know why we see this error message? I don't have
>> good feeling when we remove it, because it's annoying and we don't
>> understand why we see it.
> 
> The message is wrong because the code is wrong. Let's see:
> 
>         /* Check reason for stopping */
>         len = sizeof(data);
>         index = 0;
>         if (rvt->report_event(HAB_STATUS_ANY, index, NULL, &len) == HAB_STATUS_SUCCESS)
>                 pr_err("ERROR: Recompile with larger event data buffer (at least %d bytes)\n\n", len);
> 
> report_event() like called above will give you the first message of any
> type (HAB_STATUS_ANY) with index 0. It will do so successfully, so it
> returns HAB_STATUS_SUCCESS.
> 
> &len on entry means the length of the buffer (here sizeof(data), large
> enough). &len on exit is the length of the actual message returned. If
> &len is smaller than on entry it means the message buffer was big
> enough. If it's bigger, then we must increase the buffer size and call
> again.
> 
> The message buffer is big enough, so report_event copies the buffer
> and returns 36 bytes were copied, but we answer with "Error: Recompile
> with a larger event data buffer".

I see, this means since:

| e7c33540d0c0 i.MX: HABv4: Reset index variable after error type

the check is even more broken and fires every time we have at least one
warning and/or error.

So we can either remove the check/message with a proper commit log or
change the code to semething like:

> diff --git a/drivers/hab/habv4.c b/drivers/hab/habv4.c
> index e3c1de1a4dbe..013cdea1bc43 100644
> --- a/drivers/hab/habv4.c
> +++ b/drivers/hab/habv4.c
> @@ -507,7 +507,7 @@ static int habv4_get_status(const struct habv4_rvt *rvt)
>  {
>         uint8_t data[256];
>         uint32_t len;
> -       uint32_t index = 0;
> +       uint32_t index = 0, done = 0;
>         enum hab_status status;
>         enum hab_config config = 0x0;
>         enum hab_state state = 0x0;
> @@ -542,6 +542,7 @@ static int habv4_get_status(const struct habv4_rvt *rvt)
>  
>                 len = sizeof(data);
>                 index++;
> +               done++;
>         }
>  
>         len = sizeof(data);
> @@ -553,13 +554,13 @@ static int habv4_get_status(const struct habv4_rvt *rvt)
>                 habv4_display_event(data, len);
>                 len = sizeof(data);
>                 index++;
> +               done++;
>         }
>  
> -       /* Check reason for stopping */
> +       /* Check if we've handled every event */
>         len = sizeof(data);
> -       index = 0;
> -       if (rvt->report_event(HAB_STATUS_ANY, index, NULL, &len) == HAB_STATUS_SUCCESS)
> -               pr_err("ERROR: Recompile with larger event data buffer (at least %d bytes)\n\n", len);
> +       if (rvt->report_event(HAB_STATUS_ANY, done, NULL, &len) == HAB_STATUS_SUCCESS)
> +               pr_err("ERROR: unhandled HAB event!\n\n", len);
>  
>         return -EPERM;
>  }

Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde           |
Embedded Linux                   | https://www.pengutronix.de  |
Vertretung West/Dortmund         | Phone: +49-231-2826-924     |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-5555 |


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 149 bytes --]

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox