Re: [PATCH v2 1/3] tools:iio:iio_event_monitor: check if event is fully read

[Hartmut Knaack <knaack.h@gmx.de>]

Daniel Baluta schrieb am 10.06.2015 um 22:16:
> On Wed, Jun 10, 2015 at 10:51 PM, Hartmut Knaack <knaack.h@gmx.de> wrote:
>> Check that the read event is of the expected size.
>>
>> Signed-off-by: Hartmut Knaack <knaack.h@gmx.de>
>> ---
>>  tools/iio/iio_event_monitor.c | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/tools/iio/iio_event_monitor.c b/tools/iio/iio_event_monitor.c
>> index 016760e..2559fba 100644
>> --- a/tools/iio/iio_event_monitor.c
>> +++ b/tools/iio/iio_event_monitor.c
>> @@ -299,6 +299,12 @@ int main(int argc, char **argv)
>>                         }
>>                 }
>>
>> +               if (ret != sizeof(event)) {
>> +                       printf("Reading event failed!\n");
>> +                       ret = -EIO;
>> +                       break;
>> +               }
>> +
> 
> For a test program this should be good enough, not sure if it does make sense
> to read until we get all data.

Not sure, if I get your intention here. But as Jonathan suggested, something
went seriously wrong, if the sizes don't match here. Reading further, until
the desired size is reached could result in receiving the missing data, or
getting already partial data from the next event. I would leave it as is for
now and see if anyone ever gets this error. Then we can still dig deeper.

> 
> Also, we should print the error at stderr.
> 

I basically agree on this one, but since the majority of the error messages
in this file are currently just printed to stdout, I would prefer to take it
as is (for consistency) and have a follow-up patch to send all error messages
to stderr. 

Thanks for your input.
Hartmut

>>                 print_event(&event);
>>         }
> 
> thanks,
> Daniel.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>