On Sat, Jun 13, 2015 at 1:00 PM, Hartmut Knaack <knaack.h@xxxxxx> wrote:
> 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.
Ok then, I'll wait until this patchset gets applied and then make the
requested changes.
>> 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 for the tip, I'll make sure to use it when appropriate.
Cristina
> 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
