Hi Robin, On Fri, Nov 13, 2020 at 03:16:07PM +0100, Robin van der Gracht wrote: > If index is reset and the event record pointer is NULL the size of the > indexed event will be written to 'len'. > > Currently this results in always printing the buffer overflow error if > there is a HAB event. The index shouldn't be reset to get the size of the > 'next' unhandled event. This can occur if there are more events of a single > type than the event buffer (data) can hold. > > Telling the user to recompile with an incomplete additional size > suggestion seems useless so I changed it to something more generic. > > Signed-off-by: Robin van der Gracht <[email protected]> > --- > drivers/hab/habv4.c | 11 ++++++----- > 1 file changed, 6 insertions(+), 5 deletions(-) > > diff --git a/drivers/hab/habv4.c b/drivers/hab/habv4.c > index e94f82754..3df1d8d3d 100644 > --- a/drivers/hab/habv4.c > +++ b/drivers/hab/habv4.c > @@ -504,7 +504,7 @@ static int habv4_get_status(const struct habv4_rvt *rvt) > { > uint8_t data[256]; > uint32_t len; > - uint32_t index = 0; > + uint32_t cnt, index = 0; > enum hab_status status; > enum hab_config config = 0x0; > enum hab_state state = 0x0; > @@ -540,6 +540,7 @@ static int habv4_get_status(const struct habv4_rvt *rvt) > len = sizeof(data); > index++; > } > + cnt = index; > > len = sizeof(data); > index = 0; > @@ -551,12 +552,12 @@ static int habv4_get_status(const struct habv4_rvt *rvt) > len = sizeof(data); > index++; > } > + cnt += index; > > - /* Check reason for stopping */ > + /* Check if there are more events */ > 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);
You are right, this is wrong. This call will always return HAB_STATUS_SUCCESS and return the length of the message in len. So yes, this message is always printed which doesn't make sense. I think that this whole stuff could be imrpoved. First of all semantics of the report_event function is meant in a way that first a NULL pointer is used for the buffer so that the function reports the number of bytes needed for this event. In the second pass it can be called with a buffer of exactly this length. Second I don't think it's good to separate the events parsing into HAB_STATUS_ERROR and HAB_STATUS_WARNING. If we call report_event with HAB_STATUS_ANY instead we'll get the events in the order they occured which probably better helps to make sense of the events. I've prepared a patch for this, but I currently can't test it, so it would be great if you could give it a shot. I'll send it out in a moment. Sascha -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | _______________________________________________ barebox mailing list [email protected] http://lists.infradead.org/mailman/listinfo/barebox
