Daniel Baluta schrieb am 13.06.2015 um 10:46:
> On Sat, Jun 13, 2015 at 11:36 AM, Hartmut Knaack <knaack.h@xxxxxx> wrote:
>> Daniel Baluta schrieb am 10.06.2015 um 22:16:
>>> On Wed, Jun 10, 2015 at 10:51 PM, Hartmut Knaack <knaack.h@xxxxxx> wrote:
>>>> Check that the read event is of the expected size.
>>>>
>>>> Signed-off-by: Hartmut Knaack <knaack.h@xxxxxx>
>>>> ---
>>>> 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.
>
> Ok.
>
>>
>>>
>>> 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.
>
> I've also noticed this. So I agree with you, a follow up patch would be better.
>
> Cristina can do this, if you haven't already started working on it.
>
Sounds good to me. I would have waited with such changes until this set got
applied, anyway.
> As a part of Outreachy Program [1] we are also investigating these user space
> tools plus the IIO dummy driver which hopefully at the end it will be moved
> out of staging.
>
>From my experience so far, I would regard it a good idea to chop up the
workflow to get code out of staging:
- get all the changes you have in mind applied
- announce your intention to move it out in a reasonable time (2 weeks?), so
people who mind will feel pressured to have a look at the code and fix
issues in staging
- move out smoothly ;-)
Also, making use of gits copy detection is a good idea to avoid huge patches,
which basically just copy/move a file without changes. Makes the reviewers
life much easier ;-)
Thanks,
Hartmut
>
> Daniel.
>
> [1] http://kernelnewbies.org/OutreachyIntro
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
